As a programmer, I want strong modular boundaries (use Java modules introduced by project jigsaw) #208
Comments
|
Original Redmine Comment Rename to not be relative |
|
Original Redmine Comment Again clarify title. "Jigsaw" was a project, "Modules" are the construct. |
|
Original Redmine Comment Should improve startup performance as well. |
|
Original Redmine Comment Bump. Probably can do it incrementally. Progress in the gradle build in the last few weeks, moving to the @implementation@ and @api@ declarations, which are kind of related to the modular boundaries and descriptions. |
|
Original Redmine Comment This should be done regardless of or prior to using native-image or server mode in WRES because this will improve performance in all three situations (the current situation or mode being the third). |
|
Original Redmine Comment The order in which to do it, from simplest leaf modules with fewest deps up to complex trunk modules with most deps:
|
|
Original Redmine Comment I added this to the @wres-messages@ section of the @build.gradle@: I added a @wres-messages/src/main/java/module-info.java@ with these contents: Then I tried building the jar: Good. It seems to have had an effect and gradle tried to build it as a module. @wres.messages@ indeed uses @org.slf4j@ so I need to add that to the @module-info.java@. |
|
Original Redmine Comment After adding @requires org.slf4j;@ to the @module-info.java@, it built the jar: Is that it? If so, great! |
|
Original Redmine Comment What's inside the jar? Nice. Now on to @wres-worker@... |
|
Original Redmine Comment Hmm, no such error when building the jar for @wres-worker@ when missing @com.rabbitmq@. But after doing a clean, OK, it had the expected errors. |
|
Original Redmine Comment Here is the @module-info.java@ for @wres-worker@: It doesn't export anything because it's a top-level application. It only uses this handful of libraries. What about using @jlink@ now? Can gradle do that for me? Looking... |
|
Original Redmine Comment A nice note here at https://docs.gradle.org/6.9.1/userguide/java_library_plugin.html#declaring_module_dependencies
|
|
Original Redmine Comment Should @wres-messages@ call @protobuf@ an @api@ dependency? Maybe. Does @wres-worker@ import/use any classes from @protobuf@ that are not @wres.messages@ classes? Yeah, a couple of spots it uses a timestamp and an exception. Edit: and it already declares protobuf an @api@ dependency, so we need to make the Java Module Directive in @module-info.java@ match the declaration in @build.gradle@, i.e. to be @requires transitive@. |
|
Original Redmine Comment When making it match, protobuf is reported as an automatic module: Why the warning?
So we probably do not want @requires transitive@ for protobuf, because that would add everything that protobuf has and uses? I don't quite have the picture in my head but I trust the warning. Going back to @requires@. |
|
Original Redmine Comment I see this gradle plugin for jlink: https://plugins.gradle.org/plugin/org.beryx.jlink It warns that it is a complex plugin and to read the docs first. Perhaps I should try jlink by hand first with @wres-worker@ which is a relatively simple application. |
|
Original Redmine Comment Before that, a @JavaDoc@ task failure when cleaning and compiling the whole tree: Edit: I had previously excluded generated code javadocs: Edit4: removing that exclusion from submodules and from wres-messages resolves the immediate issue but then creates a different issue of annoying warnings from lack of complete javadocs from protoc. |
|
Original Redmine Comment It may be a high bar to use jlink: |
|
Original Redmine Comment There are facilities for generating a @module-info.java@ for 3rd-party jars, though. @jdeps@ etc. |
|
Original Redmine Comment Back to that gradle plugin:
|
|
Original Redmine Comment I added the plugin to @wres-worker@ and ran the @jlink@ task which on the surface worked fine: However, when running the resulting script: However, this probably only means we need to add @org.slf4j@ to the merged module. The plugin takes all "automatic" modules and merges them together into a mega-module proper. So perhaps I can have the additional dep added somewhere. |
|
Original Redmine Comment That's a pretty nice sized image, though: 95MiB including java runtime. Ubi8 is around 216MiB sans java runtime, the @wres-worker@ image is 583MiB including the 2.8MiB of application and by deduction 364MiB of JVM. So an ubi8 image plus this jlink wres-worker image would presumably be around 311MiB instead of 583MiB. |
|
Original Redmine Comment The jlink plugin includes a handy @suggestMergedModuleInfo@ gradle task, which I believe is showing what it generated for the unnamed module subsuming all non-truly-jpms-modules: That's a pretty short list. |
|
Original Redmine Comment Edit: I added this jlink block to the @wres-worker@ block of @build.gradle@ to specify the @org.slf4j@ dependency: It seems to have partially worked, maybe we need to include logback, though: That seems to be about twice as fast to get off the ground as this (default script): |
|
Original Redmine Comment Hmm, module paths are different from classpaths, and attempting to add @ requires ch.qos.logback;@ shows an error even in my IDE, same at @jar@ time, same for @ch.qos.logback.classic@ and/or @ch.qos.logback.core@. I have logback-classic specified as a @runtimeOnly@ dependency in gradle but to get it in the module I suppose it has to be compile time. |
|
Original Redmine Comment Something like this, perhaps: beryx/badass-jlink-plugin#191 |
|
Original Redmine Comment Nice. You can use the mergedModule to help include these runtime dependencies in the resulting image. After adding @requiresTransitive 'ch.qos.logback.classic'@ to the @mergedModule@ block in the @jlink@ block exposed by the @org.beryx.jlink@ plugin, it seems logback is there (and increases the runtime a little too): So using the jlink image it only is shaving off about 1/5 of a second from startup. The bigger advantages will be the smaller docker images, better modularity, security. Edit: and it is still not par because I don't see the logback.xml being applied properly here, so that string formatting could add back another 100ms, who knows? |
|
Original Redmine Comment Better answer probably here, mentioning that logback uses SPI: https://stackoverflow.com/questions/54777923/logback-in-a-java-9-modular-application-not-working |
|
Original Redmine Comment Or just adding @requires@ works too. I'm not sure that this is the best answer, though: Edit: and on clean build this does not suffice. |
|
Original Redmine Comment Adding @ forceMerge 'slf4j'@ was not enough to bring logback on either. I tried this, but it causes a different error, which makes sense on second glance because it is forcing inclusion of all dependencies of the declared modules: Error: How about add extra dependencies? Like this: It builds, but logback does not come in, and a different issue occurs: Given the multitude of options available with this plugin I am pretty confident it is a matter of finding the exact right incantation. |
|
Original Redmine Comment Here is the @jlink@ command the plugin generates (found by adding @--debug@ to the @Gradlew@ command): Do we want it added as a module there in @--add-modules@? Perhaps finding the right @jlink@ command first and then figuring out how to get the plugin to do that is better. |
|
Original Redmine Comment https://github.com/nebula-plugins/gradle-aggregate-javadocs-plugin is marked archived by the owner. Well, it still seemed to work. |
|
Original Redmine Comment I tried @io.freefair.aggregate-javadoc@ 6.4.3 but it didn't let me run @./gradlew tasks@. Then I tried @me.julb.gradleplugins.aggregatejavadoc@ and it failed with the same error as the netflix (one we had earlier). This one lets me exclude whole subprojects but I don't really want to do that either. I would like them all included. |
|
Original Redmine Comment This one from spring @ id "io.spring.javadoc-aggregate" version "0.0.1"@ gives an interesting verbose error: |
|
Original Redmine Comment It looks like the @JavaDoc@ command is conscious of modules, etc., so perhaps it has the ability to make an aggregated view starting with the top module. |
|
Original Redmine Comment And javadoc needs the @module-info.java@ or else it complains. If I take the @wres-worker@ @build/tmp/javadoc/javadoc.options@ and pass those args to @JavaDoc@, no problem, but if I remove the @module-info.java@ from the list of sources to that same command, it complains. |
|
Original Redmine Comment What are the next jars to modularize? Or @wres-grid@, @wres-system@, @wres-util@ look like decent candidates based on size and probably the dep tree too. |
|
Original Redmine Comment @wres-util@ depends on @wres-datamodel@. @wres-datamodel@ depends on @wres-config@ and @wres-statistics@. Wait, what? Why does @wres-datamodel@ depend on @wres-statistics@?? |
|
Original Redmine Comment Ah. @wres-statistics@ has all the data model for statistics, i.e. protobuf descriptions, etc., and then @wres-datamodel@ uses those. |
|
Original Redmine Comment @wres-statistics@ depends on @wres-config@. Or as gradle puts it: So the natural order would be:
|
|
Original Redmine Comment @wres-statistics@ depends on @wres-config@? Will have to take a closer look at that. It shouldn't, as I recall. It is basically a bunch of generated classes (java bindings for protobufs), plus one tiny utility class. |
|
Original Redmine Comment Yeah, clean-up on aisle @wres-statistics@. I guess there was more in there at some point. |
|
Original Redmine Comment commit:wres|fde40c13c9e6a433a1a5c016be7bb9da8cd927c2. |
|
Original Redmine Comment |
|
Original Redmine Comment And right away, we find that @wres-config@ has the most baggage of any of 'em, XML and jaxb, xjc, and the lot. |
|
Original Redmine Comment Oh, nice. That means @wres-statistics@ can be modularized easily. |
|
Original Redmine Comment Customer issue #104204 and associated issue #104251 |
|
Original Redmine Comment commit:eb11e4b5a8f1af2f86b2490a5064b24285c43459 makes @wres-statistics@ a JPMS module. |
|
Original Redmine Comment @wres-config@ is not as straightforward and it seems related to all these old jaxb2 dependencies. But it looks like progress is happening toward use of the jakarta namespace, e.g. highsource/jaxb2-basics#134
|
|
Original Redmine Comment I get these warnings when running in the IDE. I am not bothered, because they are warnings (edit: and don't lead to errors that prevent success) and I don't see them when running normally. Still, reporting here. |
|
Original Redmine Comment Good to know. It has been so long since I have tried running within IntelliJ IDEA that it looks like my old configs won't run. |
|
Original Redmine Comment I was able to run scenario505 after some finagling. But that was with IntelliJ IDEA 2021.3.2. I'm updating now. |
|
Original Redmine Comment I can't find that message (@warning: Unknown module: wres.statistics specified to --patch-module@) locally, is it when you build or run? I cannot easily display the background build task when running either, so is it showing up there? I have @IntelliJ IDEA 2021.3.3 (Community Edition)@ using @runtime version: 11.0.15+10-LTS amd64@ @vm: OpenJDK 64-Bit Server VM by Azul Systems, Inc.@ on GNU/Linux. |
|
Original Redmine Comment Maybe a @clean@ and then @installDist@? Does it still show up then? |
|
Original Redmine Comment No, build is fine, this is when I run. I wouldn't sweat it, there is no issue when I run the app using the run script, so it is unlikely to be our software. I cleaned, yes. I can probably track it down, but I just have zero motivation to do so (vs. other things) because nothing is broken. edit: in other words, I wasn't posting for help - although I appreciate the suggestions - more as a record of an observation. |
|
Original Redmine Comment Fair enough. |
|
Original Redmine Comment Noting again that this work broke the @aggregateJavadocs@ task, which means that we do not currently have aggregated javadocs associated with a build. |
|
Original Redmine Comment James wrote:
@io.freefair.aggregate-javadoc@ looks like a good option, but only when we upgrade to gradle 7.4.2 or later: https://docs.freefair.io/gradle-plugins/current/reference/#_system_requirements |
|
Original Redmine Comment See #62732-80. Worth a try in due course. It would be nice to bring back our aggregated docs. |
|
Original Redmine Comment James wrote:
Although this is a cause for pessimism: We'll see. Should probably break into a separate ticket. |
|
Original Redmine Comment Broken out the javadoc issue, back to the backlog for this one. |
Author Name: Jesse (Jesse)
Original Redmine Issue: 62732, https://vlab.noaa.gov/redmine/issues/62732
Original Date: 2019-04-18
Given a build of WRES
When try to use the various modules (jars) in another program or another module
Then there should be clear and strong boundaries enforced by the system
Many people worked for a long time to afford this possibility with Java 9 in "Project Jigsaw":https://openjdk.java.net/projects/jigsaw/ so now that we are past Java 8 we can start to make use of these features.
Related issue(s): #102
Redmine related issue(s): 111518, 111532, 119337
The text was updated successfully, but these errors were encountered: