FWD » Core Development » Runtime Infrastructure

Java 17 Release Notes
Java 17 Features

As of Sept 2021, Java 17 is the latest LTS (long term support) release. I'd like to take a quick look at the effort of moving to Java 17 as well as any performance benefit to doing so.

Java 11 is an LTS release, as is Java 8 (which are are on at the moment). In #5567, we are close to merging the changes back and enabling support for both Java 8 and Java 11 in branch 3821c (soon to be in trunk of FWD v4). The problem here is that Java 11 seems to be consistently slower than Java 8. By ChUI testing, the difference is 5-30% and in one customer's appserver testing it was 10-20%. That is a large enough problem that most customers won't want to use it. If Java 17 is as fast or faster than Java 8, then it would be a priority to move to 17 unless there was a functional reason otherwise.

Java 17 has more changes to the underlying API and I'm pretty sure our code base will require changes to enable support. These changes will be incompatible with Java 8/11.

Tomasz: Please go ahead and start work on the estimate. This is in parallel to your other work, so don't consume all of your time with this. Please keep a running summary of the changes expected as you identify them.

Regarding what you might see as a minor devops item... version information.

Currently version.properties contains something like:

major=4
minor=0
release=0

#repo, branch and rev are attempted to be read from code repository if not defined here
repo=p2j
branch=3821c
rev=14349

#if desired the default version format can be changed here
#format=<major>.<minor>.<release>_<repo>_<branch>_<rev>

I utilize this information when creating the archive target such that we know important details about the distributed files. However, we don't have any way to indicate which version of Java was used to build FWD.

Any thought given to this? It gets even worse when we consider now Java 17... so at some near future point, we could be dealing with 3 varieties. Am I making much ado about nothing?

If Java 17 is as fast or faster than Java 8, then we will never use Java 8 or Java 11 again. Usually, we only would support a single version of Java. Let's defer this concern for now.

Its not just about Java version, but IMO its more about a whole execution environment. E.g. different database types, database engine versions, OS etc

The OS version matters for a FWD build but we (as yet) have no differences by any other part of the environment. In particular, the database side is independent.

• rename com.goldencode.p2j.persist.Record to com.goldencode.p2j.persist.AbstractRecord
• add explicit import com.goldencode.p2j.persist.Record; to every class where "error: reference to Record is ambiguous" happens

I would guess the second is preferable, but this is really Eric's call.

I've tried to move a bit and have done initial migration to gradle 7.5.1 and tried to build FWD with Java 17.
Just to feel it I've also tried to upgrade dependencies to their latest versions but I gave up after 2-3 components, as performing that is complex and takes more time, I presumed providing initial estimate and plan has higher priority than actual upgrade at this stage. I guess I'd need 2 days for providing usable estimate.

Regarding steps: I'd like to add partial dependencies upgrade step, as it seems it is be not very costly.
From what I see (until now) upgrading dependencies' versions does not affect downwards compatibility - the latest versions of dependent components stay Java 8 downwards compatible.

For now a plan looks like following:

• Build environment upgrade to Java 17 with build tools upgrade and partial dependencies upgrade (only necessary upgrades)
  • Migrate to gradle 7.5.1 (does not include migrating to java plugin) - (done)
  • Update all dependencies (jars, scripts) to the latest versions working with current FWD code base to get FWD@Java 17 working configuration if feasible - 1d
• Update all (or as many as can be done without changing Java versions) dependencies to their latest levels. - this is something I cant estimate fully at this stage - there are too many direct and indirect dependencies that may affect the process when upgraded and I need more time for this.
  • execute full dependencies analysis (find latest versions, check for compatibility etc) - 2d
  • Rework FWD code to allow usage with latest dependencies versions - ?d
  • create sub-tasks for upgrading components and FWD per case if needed (to implement larger changes) - ?d
• Testing
  • regression testing (java 8, java 11, java 17) and results analysis - Effort: 2d, Elapsed: 6d due to tests execution time
  • hotel_chui app,hotel_gui app - less then 1d
  • performance testing (Java 8 vs Java 17) - 2d (getting test base, tests execution, tests results analysis)
• Merge Java 8/11 build configuration in 6692 (Java 8/11 stays on current dependencies' versions, Java 17 uses latest versions) - Effort: 1d
• Merge back 6692 into 3821c - 1d
• Post-merge support - 2d reserve

It would also help me if you specify how much time I can spend on this task in total,weekly or monthly.

Restarting the tasks.
After rebasing to latest trunk refreshing gradle configuration.

There are 27 warnings mainly related with code deprecation. I will check them one by one and try to clear them minimizing suppressing.

[ant:javac] Note: Some input files use unchecked or unsafe operations.
[ant:javac] Note: Recompile with -Xlint:unchecked for details.
[ant:javac] 27 warnings

Regarding Java 17 and modules - there are few places I need to add --add-opens JVM option to allow access of base module internals.

Greg, are there any broader guidelines regarding FWD and Java Modules (JPMS) ?

Greg, are there any broader guidelines regarding FWD and Java Modules (JPMS) ?

Not yet. I am open to recommendations.

I've cleaned 6692 from compiler warnings by updating FWD code.
There were two cases where I had to add deprecated note - in order to keep compatibility with Java 8 I couldn't migrate following warnings:

[ant:javac] /home/tjd/projects/p2j_6692a/src/com/goldencode/p2j/persist/orm/DataObjectFactory.java:370: warning: [deprecation] isAccessible() in AccessibleObject has been deprecated
[ant:javac]                if (!field.isAccessible())
[ant:javac] /home/tjd/projects/p2j_6692a/src/com/goldencode/p2j/util/logging/CentralLogRecord.java:222: warning: [deprecation] setMillis(long) in LogRecord has been deprecated
[ant:javac]       cl.setMillis(this.millis);

I've pushed 6692a/14923, please review.

Next I will create a new branch 6692b which will be 6692a rebased to 6667i for regression and performance testing.

Tomasz Domin wrote:

Regarding Java 17 and modules - there are few places I need to add --add-opens JVM option to allow access of base module internals.

Greg, are there any broader guidelines regarding FWD and Java Modules (JPMS) ?

After starting regression tests which is more or less the real application working on FWD I get following exceptions:

Caused by: java.lang.IllegalAccessError: class com.goldencode.p2j.persist.BufferImplMethodAccess tried to access protected method 'boolean com.goldencode.p2j.persist.BufferImpl.checkExtentRange(int, int)' (com.goldencode.p2j.persist.BufferImplMethodAccess is in unnamed module of loader com.esotericsoftware.reflectasm.AccessClassLoader @47404bea; com.goldencode.p2j.persist.BufferImpl is in unnamed module of loader 'app')
Caused by: java.lang.IllegalAccessError: class com.goldencode.p2j.persist.BufferImplMethodAccess tried to access protected method 'void com.goldencode.p2j.persist.BufferImpl.setQuery(com.goldencode.p2j.persist.QueryWrapper)' (com.goldencode.p2j.persist.BufferImplMethodAccess is in unnamed module of loader com.esotericsoftware.reflectasm.AccessClassLoader @47404bea; com.goldencode.p2j.persist.BufferImpl is in unnamed module of loader 'app')
Caused by: java.lang.RuntimeException: java.lang.IllegalAccessError: class com.goldencode.p2j.persist.BufferImplMethodAccess tried to access protected method 'boolean com.goldencode.p2j.persist.BufferImpl.checkExtentRange(int, int)' (com.goldencode.p2j.persist.BufferImplMethodAccess is in unnamed module of loader com.esotericsoftware.reflectasm.AccessClassLoader @47404bea; com.goldencode.p2j.persist.BufferImpl is in unnamed module of loader 'app')
Caused by: java.lang.RuntimeException: java.lang.IllegalAccessError: class com.goldencode.p2j.persist.BufferImplMethodAccess tried to access protected method 'void com.goldencode.p2j.persist.BufferImpl.setQuery(com.goldencode.p2j.persist.QueryWrapper)' (com.goldencode.p2j.persist.BufferImplMethodAccess is in unnamed module of loader com.esotericsoftware.reflectasm.AccessClassLoader @47404bea; com.goldencode.p2j.persist.BufferImpl is in unnamed module of loader 'app')
java.lang.IllegalAccessError: class com.goldencode.p2j.persist.BufferImplMethodAccess tried to access protected method 'boolean com.goldencode.p2j.persist.BufferImpl.checkExtentRange(int, int)' (com.goldencode.p2j.persist.BufferImplMethodAccess is in unnamed module of loader com.esotericsoftware.reflectasm.AccessClassLoader @47404bea; com.goldencode.p2j.persist.BufferImpl is in unnamed module of loader 'app')
java.lang.IllegalAccessError: class com.goldencode.p2j.persist.BufferImplMethodAccess tried to access protected method 'void com.goldencode.p2j.persist.BufferImpl.setQuery(com.goldencode.p2j.persist.QueryWrapper)' (com.goldencode.p2j.persist.BufferImplMethodAccess is in unnamed module of loader com.esotericsoftware.reflectasm.AccessClassLoader @47404bea; com.goldencode.p2j.persist.BufferImpl is in unnamed module of loader 'app')
java.lang.RuntimeException: java.lang.IllegalAccessError: class com.goldencode.p2j.persist.BufferImplMethodAccess tried to access protected method 'boolean com.goldencode.p2j.persist.BufferImpl.checkExtentRange(int, int)' (com.goldencode.p2j.persist.BufferImplMethodAccess is in unnamed module of loader com.esotericsoftware.reflectasm.AccessClassLoader @47404bea; com.goldencode.p2j.persist.BufferImpl is in unnamed module of loader 'app')

java.lang.reflect.InaccessibleObjectException: Unable to make field private transient boolean java.util.logging.LogRecord.needToInferCaller accessible: module java.logging does not "opens java.util.logging" to unnamed module @2cd76f31

For now I will try with --add-opens, but sooner or later these need to fixed by an implementation correction or a proper module packaging.

Tomasz Domin wrote:

Tomasz Domin wrote:
After starting regression tests which is more or less the real application working on FWD I get following exceptions:
[...]

These were fixed by updating signatures for checkExtentRange and setQuery to public from protected to enable access from generated code.

Plus the old one:
[...]

That was already fixed by adding --add-opens java.logging/java.util.logging=ALL-UNNAMED". Unfortunately that needs to be added to every FWD application's command line.

While regression testing I found another IllegalAccessException:

java.lang.IllegalAccessError: failed to access class com.goldencode.p2j.net.Queue from class com.goldencode.p2j.net.RouterSessionManagerMethodAccess (com.goldencode.p2j.net.Queue is in unnamed module of loader 'app'; com.goldencode.p2j.net.RouterSessionManagerMethodAccess is in unnamed module of loader com.esotericsoftware.reflectasm.AccessClassLoader @30990c1b)
        at com.goldencode.p2j.net.RouterSessionManagerMethodAccess.invoke(Unknown Source)
        at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:156)
        at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:788)
        at com.goldencode.p2j.net.Dispatcher.dispatch(Dispatcher.java:921)
        at com.goldencode.p2j.net.Dispatcher$DispatcherStub.run(Dispatcher.java:1740)
        at java.base/java.lang.Thread.run(Thread.java:833)

Tomasz Domin wrote:

That was already fixed by adding --add-opens java.logging/java.util.logging=ALL-UNNAMED". Unfortunately that needs to be added to every FWD application's command line.

I've made a fix to CentralLogger.java removing the need for adding --add-opens java.logging/java.util.logging=ALL-UNNAMED" to every server startup.
Basically now application startup scripts should support Java 17 if they already support Java 11, no new changes to startup script would be needed.

Committed Hotel_ChUI_Demo_Application change revision 95 which brings back support for Java 11 and Java 17.
The change only applies to scripts and build files, no code was changed/updated.

Committed Hotel_Gui_Demo_Application change revision 321 that brings back option to run it under Java 11 and Java 17.
Tested Swing GUI Client mode , Embedded Web Client and Virtual Desktop Web Client mode - all are working correctly under Java 17 (FWD branch @6692a/14964)

Tested Swing GUI Client mode , Embedded Web Client and Virtual Desktop Web Client mode - all are working correctly under Java 17 (FWD branch @6692a/14964)

Doesn't embedded mode require the change from #8253?

Greg Shah wrote:

Tested Swing GUI Client mode , Embedded Web Client and Virtual Desktop Web Client mode - all are working correctly under Java 17 (FWD branch @6692a/14964)

Doesn't embedded mode require the change from #8253?

It does, but I applied it temporarily for Java 17 testing as a kind of dirty and quick fix of blocking issue. I am not sure about side-effects of that change so I haven't committed it yet and I've created #8253 for someone more familiar with embedded mode to verify it.

Tomasz Domin wrote:

• Rework FWD code to allow usage with latest dependencies versions - ?d

Pushed branch 6692a revision 14983 that has IMHO all possible upgrades of dependencies. By "all possible" I mean updates of components to the latest non-beta and non-snapshot version of Java components that are available in public repositories reported by ./gradlew dependencyUpdates. If I have missed something important please let me know. I haven't upgraded any JS dependencies except dojotoolkit.
I've briefly tested hotel_gui with Java 17 and following clients: Swing GUI Client, Virtual Desktop Web Client, Embedded Web Client and Administration console - they all seem to work correctly.
The current report from ./gradlew dependencyUpdates

The following dependencies have later milestone versions:
 - antlr:antlr [2.7.7 -> 20030911] - false,, 2.7.7 is the latest
 - com.fasterxml.jackson.core:jackson-databind [2.13.4.2 -> 2.16.1] -> false, its net.sf.jasperreports:jasperreports:6.21.0 dependency
 - com.github.stephenc.high-scale-lib:high-scale-lib [1.1.2 -> 1.1.4] -> requires to stay at that version in a comment in build.gradle
 - com.google.guava:guava [19.0 -> 33.0.0-jre] - already updated -> false, already updated
 - commons-codec:commons-codec [1.15 -> 1.16.1] -> false, already updated, probably other component dependency
 - commons-codec:commons-codec [1.16.0 -> 1.16.1] -> false, already updated, probably other component dependency
 - commons-collections:commons-collections [3.2.2 -> 20040616] -> false, 3.2.2 is the latest
 - commons-io:commons-io [2.11.0 -> 2.15.1] -> false, already updated, probably other component dependency
 - commons-lang:commons-lang [2.4 -> 2.6] -> false, already updated, probably other component dependency
 - jakarta.xml.ws:jakarta.xml.ws-api [2.3.3 -> 4.0.1] -> cant be updated as it provided API missing in Java 17
 - javax.mail:mail [1.4 -> 1.5.0-b01] -> no need to upgrade plain API package
 - javax.servlet:javax.servlet-api [3.1.0 -> 4.0.1] -> no need to upgrade plain API package
 - javax.xml.bind:jaxb-api [2.3.0 -> 2.4.0-b180830.0359] -> no need to upgrade plain API package
 - org.apache.poi:poi-ooxml-full [5.2.2 -> 5.2.5] -> probably other component dependency, cant directly update
 - org.apache.tika:tika-core [2.9.1 -> 3.0.0-BETA] -> dont update to BETA
 - org.apache.xmlbeans:xmlbeans [5.0.3 -> 5.2.0] -> probably other component dependency, cant directly update
 - org.bouncycastle:bcmail-jdk14 [1.72 -> 1.77] -> false, already updated, probably other component dependency
 - org.bouncycastle:bcprov-jdk14 [1.72 -> 1.77] -> false, already updated, probably other component dependency
 - org.eclipse.jetty.aggregate:jetty-all [9.4.22.v20191022 -> 11.0.0.beta1] -> cant upgrade - it needs a separate effort
 - org.janusgraph:janusgraph-berkeleyje [1.1.0-20240207-121257.e172faf -> 1.1.0-20240207-164433.d3c0e11] -> irrelevant
 - org.janusgraph:janusgraph-core [1.1.0-20240201-181040.3df4f04 -> 1.1.0-20240207-164433.d3c0e11] -> irrelevant
 - org.janusgraph:janusgraph-lucene [1.1.0-20240207-121257.e172faf -> 1.1.0-20240207-164433.d3c0e11] -> irrelevant
 - org.jsoup:jsoup [1.15.1 -> 1.17.2] > false, already updated
 - org.quartz-scheduler:quartz [2.3.2 -> 2.5.0-rc1] -> do not upgrade to RC version
 - org.slf4j:slf4j-api [2.0.12 -> 2.1.0-alpha1] -> do not upgrade to alpha versions
 - org.slf4j:slf4j-jdk14 [2.0.12 -> 2.1.0-alpha1] -> do not upgrade to alpha versions

So I guess now its time do regression testing and take care of upgrading jetty in meantime.

Sergey: Can you please review our javascript dependencies and mae a list of the latest stable/non-beta and non-snapshot versions? We need to update them in this branch.

Greg Shah wrote:

Sergey: Can you please review our javascript dependencies and mae a list of the latest stable/non-beta and non-snapshot versions? We need to update them in this branch.

This list can be of a little help, most of them are javascript dependencies.

Failed to determine the latest version for the following dependencies (use --info for details):
 - adamwdraper:adamwdraper-Numeral-js
 - bootstrap:bootstrap
 - cbtree:cbtree
 - com.goldencode:fwd-h2
 - d3:d3
 - font-bundle:font-bundle
 - gettext:gettext.js
 - io.keikai:fwd-keikai
 - io.keikai:fwd-keikai-ex
 - io.keikai:fwd-keikai-model
 - io.keikai:fwd-keikai-poi
 - jquery:jquery
 - jquery:jquery-loading-overlay
 - jquery:jquery-ui
 - material-components-web:material-components-web
 - material-design-icons:material-design-icons
 - net.sourceforge.barbecue:barbecue
 - org.apiguardian:apiguardian-api
 - org.junit.jupiter:junit-jupiter-api
 - org.junit.platform:junit-platform-console
 - org.junit.platform:junit-platform-engine
 - org.junit.platform:junit-platform-suite-engine
 - org.roaringbitmap:RoaringBitmap
 - tabulator:tabulator-master
 - virtualscroller:virtualscroller
 - webcola:webcola

Ignore anything related to Keikai. Hynek manages those dependencies and they are already up to date.

Tomasz Domin wrote:

Tomasz Domin wrote:

• Rework FWD code to allow usage with latest dependencies versions - ?d

Pushed branch 6692a revision 14983 that has IMHO all possible upgrades of dependencies. By "all possible" I mean updates of components to the latest non-beta and non-snapshot version of Java components that are available in public repositories reported by ./gradlew dependencyUpdates. If I have missed something important please let me know. I haven't upgraded any JS dependencies except dojotoolkit.
I've briefly tested hotel_gui with Java 17 and following clients: Swing GUI Client, Virtual Desktop Web Client, Embedded Web Client and Administration console - they all seem to work correctly.
The current report from ./gradlew dependencyUpdates
[...]

Greg
I have a question about upgrading transitive dependencies.
There are some configuration where older components are in use then in other configuration, as newer versions are declared only in some configurations e.g. in fwdServer and not in base configuration like fwdCCS. Older components are there as they are needed by FWD dependencies for given configuration.
Shall I move forcing newer dependencies to base configuration so all dependencies versions are aligned for all configurations and we dont have different version of the same component in different configuration ?

Tomasz Domin wrote:

I had A LOT of trouble upgrading jetty uber-jar so for now I've left it alone for now.

So I guess now its time do regression testing and take care of upgrading jetty in meantime.

Greg
And I have a question about jetty upgrade.
I found a note about jetty classifier uber: But this Uber artifact is not for direct consumption, never was. It only exists to teach about jetty basics from the documentation point of view. It causes issues in latest jetty versions (not sure if minor or major :))
Are there any reasons why uber classifier is in use instead jar plus dependencies. I guess this would increase number of dependencies a lot, but are there any other reasons ?

Shall I move forcing newer dependencies to base configuration so all dependencies versions are aligned for all configurations and we dont have different version of the same component in different configuration ?

Yes, our intention is that everything is upgraded and we only use that latest version. If there is some reason to deviate from this, I want us to know exactly why that is the case.

I found a note about jetty classifier uber: But this Uber artifact is not for direct consumption, never was. It only exists to teach about jetty basics from the documentation point of view. It causes issues in latest jetty versions (not sure if minor or major :))
Are there any reasons why uber classifier is in use instead jar plus dependencies. I guess this would increase number of dependencies a lot, but are there any other reasons ?

I don't recall the reasons.

Constantin or Hynek: Do you recall?

Greg Shah wrote:

I found a note about jetty classifier uber: But this Uber artifact is not for direct consumption, never was. It only exists to teach about jetty basics from the documentation point of view. It causes issues in latest jetty versions (not sure if minor or major :))
Are there any reasons why uber classifier is in use instead jar plus dependencies. I guess this would increase number of dependencies a lot, but are there any other reasons ?

I don't recall the reasons.

Constantin or Hynek: Do you recall?

I think it was to bring down the number of dependencies.

Hynek Cihlar wrote:

Greg Shah wrote:

Constantin or Hynek: Do you recall?

I think it was to bring down the number of dependencies.

Regarding Jetty I had to take a step back as Jetty 9, 10 and 11 are basically different products with limited support for JVM versions - Jetty 9 works on Java 8, but neither Jetty 10, Jetty 11 nor Jetty 12 does. Jetty 12 requires Java 17, but there are changes to API so I'd need to make dedicated version of FWD classes, which I will skip for now. Despite Jetty 9 has no longer a public support there is a quite new version of Jetty 9 9.4.53.v20231009 - I guess 4 month old version is still OK, and after upgrading Guava dependency there are no related vulnerabilities reported.

Greg - if you a different opinion and I'd need to work on bringing Jetty 12 to FWD under JVM 17+ please let me know. I guess that would require a partial source code split into Java8/Java11 and Java18+ versions.

Despite Jetty 9 has no longer a public support there is a quite new version of Jetty 9 9.4.53.v20231009 - I guess 4 month old version is still OK, and after upgrading Guava dependency there are no related vulnerabilities reported.

What comes with the later Jetty versions besides the newer Java support?

Since Java 17 still has a negative performance impact compared with Java 8, it is hard to accept a forced move to Java 17. If the performance was not a problem, I would strongly consider it. I don't think we have any customers that would object. That would really make all of this easier.

Greg - if you a different opinion and I'd need to work on bringing Jetty 12 to FWD under JVM 17+ please let me know. I guess that would require a partial source code split into Java8/Java11 and Java18+ versions.

If we had to do this, how would you propse we could manage it? I would hope we could hide the messiness which some magic in the build process, but is nasty.

Greg Shah wrote:

Despite Jetty 9 has no longer a public support there is a quite new version of Jetty 9 9.4.53.v20231009 - I guess 4 month old version is still OK, and after upgrading Guava dependency there are no related vulnerabilities reported.

What comes with the later Jetty versions besides the newer Java support?

For Jetty 10 and 11 its support for newer servlet specifications. Jetty 12 supports all servlet specifications as its predecessors as it comes in 3 flavors. Jetty 12 is the newest so its supported and updated, which might be important in the context of application security, as Jetty 9 is in "End of Community Support" since June 2022.

Greg - if you a different opinion and I'd need to work on bringing Jetty 12 to FWD under JVM 17+ please let me know. I guess that would require a partial source code split into Java8/Java11 and Java18+ versions.

If we had to do this, how would you propse we could manage it? I would hope we could hide the messiness which some magic in the build process, but is nasty.

Maybe we should create Multi-release FWD JAR I need to think about it in the context of Java9+ modularization. I dont know yet.

When upgrading JanusGraph to one of 1.1.0 releases there seems to be a bug in tinkerpop/gremlin graph traversal - it fails with java.lang.ClassCastException: class org.janusgraph.graphdb.types.indextype.CompositeIndexTypeWrapper cannot be cast to class org.janusgraph.graphdb.types.MixedIndexType in case aggregate like max is in use on CompositeIndex. After index type for given attribute was changed to MixedIndexType exception does not show up anymore and ant callgraph terminates successfully.

Greg
It appears that javac and iajc task need more heap memory then default - when starting ant from gradle it seems it inherits its memory heap - by default gradle 7.6.4 gets only 512M heap memory.

Until now I was using fork="true"

If I don't you fork with new memory limits I'd need to :
- increase somehow ant memory limit to be able to call tasks straight from ant command line
- increase gradle heap in gradle.properties as well to make

What is preferred way of increasing memory available to ant and gradle - via increasing ant memory with ANT_OPTS or via using maxmem and fork task attributes ?
Using fork allows for setting heap requirements per task.

If using fork is a preferred way - what is reasonable heap limit for javac and iajc nowadays ? 1 GB is not enough for 6692a branch. I'd set it to 4Gb to prevent worrying about that for next few years time.

I need a branch able to run with java 17 but made from the branch 7156b, for issue #8114. Could someone with more knowledge on this transition help make this branch?

What is preferred way of increasing memory available to ant and gradle - via increasing ant memory with ANT_OPTS or via using maxmem and fork task attributes ?
Using fork allows for setting heap requirements per task.

I'll let Hynek comment here.

I do think that we cannot require that environment variables (e.g. ANT_OPTS) are set properly in order for our build to work. We should have good defaults that are applied automatically. Setting the heap in the ant task would be my preference.

Alexandru Donica wrote:

I need a branch able to run with java 17 but made from the branch 7156b, for issue #8114. Could someone with more knowledge on this transition help make this branch?

The existing FWD trunk already supported Java 11 (see Switching Between Java 8 and Java 11). Can't #8114 be worked using Java 11?

Alexandru Donica wrote:

I need a branch able to run with java 17 but made from the branch 7156b, for issue #8114. Could someone with more knowledge on this transition help make this branch?

Please checkout branch 6692a, note its work in progress and I dont think its recomended to rebase to that branch.
6692a is based on trunk/14974, so you can generate a patch with bzr diff -r 14974 and apply to your branch 7156b to make temporary update and make FWD build with Java 17.

Greg Shah wrote:

What is preferred way of increasing memory available to ant and gradle - via increasing ant memory with ANT_OPTS or via using maxmem and fork task attributes ?
Using fork allows for setting heap requirements per task.

I'll let Hynek comment here.

For Gradle I think you can do this in gradle.properties file placed in the project alongside build.gradle. But I never tried this so the mileage may vary.

Ant runs inside of the Gradle process so that's that.

Greg Shah wrote:

The existing FWD trunk already supported Java 11 (see Switching Between Java 8 and Java 11). Can't #8114 be worked using Java 11?

There is a slight improvement when using Java 17 rather than Java 11. I was tasked with trying with Java 17.

Tomasz Domin wrote:

• Merge Java 8/11 build configuration in 6692 (Java 8/11 stays on current dependencies' versions, Java 17 uses latest versions) - Effort: 1d

Well, that was not that straightforward.
I unified the upgrade and I did that for all JVM versions, as there are not only dependencies for which I did only minor updates, but also there are ones having major updates that required FWD code changes.

Greg, if you'd like to stay on old/current dependencies for Java 8 and keep latest dependencies version only for Java 11/17 please let me know, I'll restructure build.gradle

There are a lot of changes in dependencies between Java 8 trunk and Java 8 6692a. The only difference between 6692a Java 8 and 6692a Java 17 versions is only different version of AspectJ, as new versions no longer support Java 8.

There are still libraries that FWD probably needs to migrate out of like (not many of them): xercesImpl, ghost4j, reflections, relaxngDatatype - but this might be just to broad for this task.

Regarding regression testing - 6692a based on current 6667i passes regression testing for Java 17, Java 8 tests are in progress. For Java 17 I had to increase memory limit for client - from 40mb to 48 mb, otherwise some of regression tests were randomly failing. I guess memory requirements increase is partially related with a larger JVM runtime and partially for a bit bigger size of dependencies.

Sergey: Can you please review our javascript dependencies and mae a list of the latest stable/non-beta and non-snapshot versions? We need to update them in this branch.

I am still waiting for JS updates to finalize the task (and reduce number of vulnerabilities), I check what I can do.

All direct dependencies updates are listed below (left is trunk, right is 6692a for JVM 17 > for added, < for removed, | for changed):

cglib:cglib:2.1_3                                               |  cglib:cglib:3.3.0
com.esotericsoftware:reflectasm:1.11.6                          |  com.esotericsoftware:reflectasm:1.11.9
com.fasterxml.jackson.core:jackson-core:2.11.1                  |  com.fasterxml.jackson.core:jackson-core:2.16.1 
com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:2.11.1  |  com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider:2.16.1
com.fasterxml.woodstox:woodstox-core:6.2.6                      |  com.fasterxml.woodstox:woodstox-core:6.6.0 
com.github.stephenc.high-scale-lib:high-scale-lib:1.1.2         <
com.googlecode.gwt-validation:gwt-validation:2.1                <
com.google.guava:guava:19.0                                     <
com.google.guava:guava-gwt:21.0                                 |  com.google.guava:guava-gwt:33.0.0-jre
com.kohlschutter.junixsocket:junixsocket-common:2.5.2           |  com.kohlschutter.junixsocket:junixsocket-common:2.8.3
com.kohlschutter.junixsocket:junixsocket-native-common:2.5.2    |  com.kohlschutter.junixsocket:junixsocket-native-common:2.8.3
com.lowagie:itext:2.1.7                                         |  com.lowagie:itext:4.2.1
com.mchange:c3p0:0.9.5.2                                        |  com.mchange:c3p0:0.9.5.5
commons-codec:commons-codec:1.7 -> 1.10                         |  commons-codec:commons-codec:1.16.1
commons-io:commons-io:2.6                                       |  commons-io:commons-io:2.15.1
                                                                >  commons-logging:commons-logging:1.3.0
com.twelvemonkeys.common:common-image:3.1.2                     |  com.twelvemonkeys.common:common-image:3.10.1
com.twelvemonkeys.common:common-io:3.1.2                        |  com.twelvemonkeys.common:common-io:3.10.1
com.twelvemonkeys.common:common-lang:3.1.2                      |  com.twelvemonkeys.common:common-lang:3.10.1
com.twelvemonkeys.imageio:imageio-core:3.1.2                    |  com.twelvemonkeys.imageio:imageio-core:3.10.1 
com.twelvemonkeys.imageio:imageio-metadata:3.1.2                |  com.twelvemonkeys.imageio:imageio-metadata:3.10.1
de.intarsys.opensource:jPod:5.5.1                               |  de.intarsys.opensource:jPod:5.6
dom4j:dom4j:1.6 -> 1.6.1                                        |  org.dom4j:dom4j:2.1.4
                                                                >  jakarta.security.auth.message:jakarta.security.auth.message-api
                                                                >  jakarta.xml.ws:jakarta.xml.ws-api:2.3.3
net.java.dev.msv:msv-core:2010.2                                |  net.java.dev.msv:msv-core:2022.7
net.sf.ehcache:ehcache-core:2.6.2                               |  net.sf.ehcache:ehcache-core:2.6.11
net.sf.jasperreports:jasperreports:6.5.1                        |  net.sf.jasperreports:jasperreports:6.21.0
org.apache.axis2:axis2-adb:1.6.2                                |  org.apache.axis2:axis2-adb:1.8.2
org.apache.axis2:axis2-kernel:1.6.2                             |  org.apache.axis2:axis2-kernel:1.8.2
org.apache.axis2:axis2-transport-http:1.6.2                     |  org.apache.axis2:axis2-transport-http:1.8.2
org.apache.axis2:axis2-transport-local:1.6.2                    |  org.apache.axis2:axis2-transport-local:1.8.2
org.apache.bcel:bcel:5.2                                        |  org.apache.bcel:bcel:6.8.1
org.apache.commons:commons-email:1.5                            |  org.apache.commons:commons-email:1.6.0
                                                                >  org.apache.commons:commons-text:1.11.0
org.apache.httpcomponents:httpclient:4.5.1                      |  org.apache.httpcomponents:httpclient:4.5.14
org.apache.james:apache-mime4j:0.8.3                            |  org.apache.james:apache-mime4j:0.8.9

org.apache.pdfbox:fontbox:2.0.5                                 |  org.apache.pdfbox:fontbox:3.0.1 
org.apache.pdfbox:pdfbox:2.0.5                                  |  org.apache.pdfbox:pdfbox:3.0.1
                                                                >  org.apache.logging.log4j:log4j-1.2-api:2.22.1
                                                                >  org.apache.logging.log4j:log4j-core:2.22.1
org.apache.poi:poi:3.15                                         |  org.apache.poi:poi:5.2.5
org.apache.tika:tika-core:1.18                                  |  org.apache.tika:tika-core:2.9.1
org.apache.ws.commons.axiom:axiom-api:1.2.22                    |  org.apache.ws.commons.axiom:axiom-api:1.4.0 
org.apache.ws.commons.axiom:axiom-impl:1.2.22                   |  org.apache.ws.commons.axiom:axiom-impl:1.4.0 
org.apache.xmlgraphics:batik-codec:1.8                          |  org.apache.xmlgraphics:batik-codec:1.17
org.apache.xmlgraphics:batik-extension:1.8                      |  org.apache.xmlgraphics:batik-extension:1.17
org.apache.xmlgraphics:batik-gui-util:1.8                       |  org.apache.xmlgraphics:batik-gui-util:1.17
org.apache.xmlgraphics:batik-gvt:1.8                            |  org.apache.xmlgraphics:batik-gvt:1.17
org.apache.xmlgraphics:batik-svggen:1.8                         |  org.apache.xmlgraphics:batik-svggen:1.17
org.apache.xmlgraphics:batik-swing:1.8                          |  org.apache.xmlgraphics:batik-swing:1.17
org.apache.xmlgraphics:batik-transcoder:1.8                     |  org.apache.xmlgraphics:batik-transcoder:1.17
org.apache.xmlgraphics:xmlgraphics-commons:2.0                  |  org.apache.xmlgraphics:xmlgraphics-commons:2.9 
                                                                >  org.apiguardian:apiguardian-api:1.1.2
org.aspectj:aspectjrt:1.9.7                                     |  org.aspectj:aspectjrt:1.9.21
org.aspectj:aspectjtools:1.9.7                                  |  org.aspectj:aspectjtools:1.9.21
org.aspectj:aspectjweaver:1.9.7                                 |  org.aspectj:aspectjweaver:1.9.21
org.bouncycastle:bcmail-jdk15to18:1.69                          |  org.bouncycastle:bcmail-jdk18on:1.77
org.bouncycastle:bcpg-jdk15to18:1.69                            |  org.bouncycastle:bcpg-jdk18on:1.77
org.bouncycastle:bcpkix-jdk15to18:1.69                          |  org.bouncycastle:bcpkix-jdk18on:1.77
org.bouncycastle:bcprov-jdk15to18:1.69                          |  org.bouncycastle:bcprov-jdk18on:1.77
org.bouncycastle:bctls-jdk15to18:1.69                           |  org.bouncycastle:bctls-jdk18on:1.77
org.codehaus.woodstox:stax2-api:4.2.1                           |  org.codehaus.woodstox:stax2-api:4.2.2
org.dojotoolkit:dojo:1.10.0                                     |  org.dojotoolkit:dojo:1.17.3
org.eclipse.jetty.aggregate:jetty-all:9.4.22.v20191022          <
                                                                >  org.eclipse.jetty:jetty-server:9.4.53.v20231009
                                                                >  org.eclipse.jetty:jetty-webapp:9.4.53.v20231009
                                                                >  org.eclipse.jetty.websocket:websocket-server:9.4.53.v20231009
                                                                >  org.eclipse.emf:common:2.2.3
org.eclipse.jetty:jetty-proxy:9.4.22.v20191022                  |  org.eclipse.jetty:jetty-proxy:9.4.53.v20231009
                                                                >  org.freemarker:freemarker:2.3.32
                                                                |  org.glassfish:javax.json:1.1.4
org.gwtbootstrap3:gwtbootstrap3:0.9.4                           |  org.gwtbootstrap3:gwtbootstrap3:1.0.1
org.gwtbootstrap3:gwtbootstrap3-extras:0.9.4                    |  org.gwtbootstrap3:gwtbootstrap3-extras:1.0.2
org.janusgraph:janusgraph-berkeleyje:0.1.0                      |  org.janusgraph:janusgraph-berkeleyje:1.1.0-20240207-121257.e172
org.janusgraph:janusgraph-core:0.1.0                            <
org.janusgraph:janusgraph-lucene:0.1.0                          |  org.janusgraph:janusgraph-lucene:1.1.0-20240207-121257.e172faf
org.javassist:javassist:3.19.0-GA+ -> 3.19.0-GA                 |  org.javassist:javassist:3.30.2-GA
                                                                >  org.jboss.logging:jboss-logging:3.5.3.Final
org.jsoup:jsoup:1.14.3                                          |  org.jsoup:jsoup:1.17.2
org.junit.jupiter:junit-jupiter-api:5.9.0                       |  org.junit.jupiter:junit-jupiter-api:5.10.2
org.junit.platform:junit-platform-console:1.9.0                 |  org.junit.platform:junit-platform-console:1.10.2
org.junit.platform:junit-platform-engine:1.9.0                  |  org.junit.platform:junit-platform-engine:1.10.2 
org.junit.platform:junit-platform-suite-engine:1.9.0            |  org.junit.platform:junit-platform-suite-engine:1.10.2
org.mariadb.jdbc:mariadb-java-client:3.1.2                      |  org.mariadb.jdbc:mariadb-java-client:3.3.2
org.ow2.asm:asm:5.0.3                                           |  org.ow2.asm:asm:9.6
org.postgresql:postgresql:42.2.5                                |  org.postgresql:postgresql:42.7.1
org.quartz-scheduler:quartz:2.2.1                               |  org.quartz-scheduler:quartz:2.3.2
org.roaringbitmap:RoaringBitmap:0.9.40                          |  org.roaringbitmap:RoaringBitmap:1.0.1
                                                                >  org.reflections:reflections:0.10.2
org.shredzone.acme4j:acme4j-client:0.10                         <
org.shredzone.acme4j:acme4j-utils:0.10                          <
org.slf4j:jcl-over-slf4j:1.7.36                                 |  org.slf4j:slf4j-jdk14:2.0.12
org.slf4j:slf4j-api:1.7.36                                      |  org.slf4j:slf4j-api:2.0.12
xerces:xercesImpl:2.11.0                                        |  xerces:xercesImpl:2.12.2

Adding new dependencies is usually related with an update of a new transitive dependency. Removed libraries are usually brought as transitive dependency.
I've removed ACME libs completely (it was not working since Java 11 update anyway).

I have pushed Java 17 compatible branch 6692a revision 15009, which is based on a quite recent trunk revision 14990.
It has been regression tested using Java 8 and Java 17 runtimes.

Tomasz, do you have any comparison on performance between 8 and 17?

Hynek Cihlar wrote:

Tomasz, do you have any comparison on performance between 8 and 17?

OK, I found it in #8208-27.

if you'd like to stay on old/current dependencies for Java 8 and keep latest dependencies version only for Java 11/17 please let me know,

I have no interest in this, unless we find some failure than only can be resolved with the old dependencies.

What work is left on this task? Do we just need wider testing?

Greg Shah wrote:

if you'd like to stay on old/current dependencies for Java 8 and keep latest dependencies version only for Java 11/17 please let me know,

I have no interest in this, unless we find some failure than only can be resolved with the old dependencies.

I guess old components are tested better thus instability/failure risk is lower. In case there is a customer having current version of FWD 4 on production there is a risk of instabilities and a bit higher memory usage (even for Java 8).

What work is left on this task? Do we just need wider testing?

• updating fwd-imageio-bmp to 3.10.1 to due to a critical vulnerability CVE-2021-23792 (will submit the patch) - in progress, eta 2h
• updating JavaScript components - Sergey or I can do it.
• testing, testing, testing - especially JasperReports and GWT dependent code, spawner also needs some testing

I did manual tests (mainly with hotel_gui) and regression tests, but there may be parts of FWD that may be affected by subcomponent update not tested yet.

updating JavaScript components - Sergey or I can do it.

Tomasz: Please take this. Sergey is busy with bugs.

Greg - can you please upload fwd-imageio-bmp-3.10.1.jar to GCD maven repository ?

Sorry, wrong upload. fwd-imageio-bmp-3.10.1.jar is now inside a zip file that just needs to be unzipped into GCD root of maven repository.

Done.

Greg Shah wrote:

I know you've been doing some amount of testing on this. As far as I know, we only have testing and any fixes that are needed based on issues found in the testing.

• Are there any open bugs?

None I know of.

• What has been tested so far?

Standard regression tests + hotel GUI

• Is there any other work needed?

I still need to update JS libraries.

Did you test both embedded mode and virtual desktop in Hotel GUI?

Did you test call graph in Hotel GUI?

We need to test ETF and the POC. Optimally, some smoke testing in a large GUI application would also be a good idea.

Guys,

Regarding possible migration I have a question. Will we have performance degradation with Java 8 -> Java 17? Do we have some comparison estimations? I guess for some projects this could be critical point.

Yes, this can be found in #8208-18. A summary for those that can't access that task:

• Java 8 is faster than Java 11 by 21,0% on average per test
• Java 8 is faster than Java 17 by 13,3% on average per test
• Java 17 is faster than Java 11 by 5,5% on average per test

Calculations for weighted average - the longer the test the higher its weight - and the performance difference is smaller:

• Java 8 is faster than Java 11 by 12,35% on weighted average per test
• Java 8 is faster than Java 17 by 9,82% on weighted average per test
• Java 17 is faster than Java 11 by 3,32% on weighted average per test

So the difference is less visible for longer operations (tests).

The short version: expect to lose up to 13% when moving from Java 8 to Java 17. It is better than Java 11 but not as good as we would hope for.

It would be interesting to profile a test case with both Java 8 and Java 17 and see where the times differ. Having more insights into the timings we could perhaps tune up some of the JVM settings.

Greg Shah wrote:

Did you test both embedded mode and virtual desktop in Hotel GUI?

Yes, I did

Did you test call graph in Hotel GUI?

And Yes, I did :)

We need to test ETF and the POC. Optimally, some smoke testing in a large GUI application would also be a good idea.

Fully agree.

Rebased to trunk/15136, pushed into repository 6692/15156

Working on JS libraries updates.

Rebased 6692a to trunk/15146 and pushed 6692a/15146.

I started to work on JS updates, upgraded with webjars versions jquery,jquery-ui,bootstrap,d3 - tried to update matching path.
But does it really make sense ? It seems JS libraries are managed by client application, I tried to update libraries but got nothing but failures, as web resource path were not matching (specific internal resource paths need to be kept).

As an example please check web_partial_login_embedded.html from hotel_gui:

      <link rel="stylesheet" href="embedded/dojo-toolkit/dijit/themes/claro/claro.css" type="text/css">
      <link rel="stylesheet" href="embedded/bootstrap/css/bootstrap.min.css" type="text/css">
      <link rel="stylesheet" href="embedded/jquery-ui/jquery-ui.min.css" type="text/css">
      <link rel="stylesheet" href="embedded/tabulator/tabulator.css" type="text/css">
      <link rel="stylesheet" href="embedded/custom/ehotel.css" type="text/css">
      <script type="text/javascript" src="embedded/jquery/jquery-3.2.1.js"></script>
      <script type="text/javascript" src="embedded/jquery-ui/jquery-ui.min.js"></script>
      <script type="text/javascript" src="embedded/bootstrap/js/bootstrap.min.js"></script>
      <script type="text/javascript" src="embedded/custom/moment-2.17.1/moment.js"></script>
      <script type="text/javascript" src="embedded/d3/d3.js"></script>
      <!-- http://bl.ocks.org/davegotz/bd54b56723c154d25eedde6504d30ad7 -->
      <script type="text/javascript" src="embedded/custom/d3-tip/d3.tip.js"></script>
      <script type="text/javascript" src="embedded/tabulator/tabulator.js"></script>

Note all libraries are included in client application. I guess this is a kind of a standard FWD JS header.
I guess they should be included by a standard script like fwd_sdk.js instead, so all needed version are loaded by FWD framework.

For now I'll stop here unless instructed otherwise.

I prefer not to leave old dependencies in the code. On the other hand, I don't want to delay this branch since many customers are waiting.

Tomasz: What do you expect is the effort for the JS dependencies?

Hynek: What do you think?

Greg Shah wrote:

Hynek: What do you think?

I like the idea of having the dependencies in fwd_sdk.js. Moving them from the individual project in the core FWD source tree will simplify dependency management and deployment.

Greg Shah wrote:

I prefer not to leave old dependencies in the code. On the other hand, I don't want to delay this branch since many customers are waiting.

Tomasz: What do you expect is the effort for the JS dependencies?

Its hard to say, I need to look in different places and I am not proficient in JS.

Hynek: What would you estimate for the effort?

Greg Shah wrote:

Hynek: What would you estimate for the effort?

I would say up to two days. The JS dependencies are indeed a bit messy.

• JS dependencies are mostly served by WebResourceHandler, see GuiWebDriver.init where it is setup
• embedded resources are served by EmbeddedWebHandler
• Report Server has its own mechanism, see ThirdPartyDependenciesHandler

Hynek Cihlar wrote:

Greg Shah wrote:

Hynek: What would you estimate for the effort?

I would say up to two days. The JS dependencies are indeed a bit messy.

Some notes:

• JS dependencies are mostly served by WebResourceHandler, see GuiWebDriver.init where it is setup
• embedded resources are served by EmbeddedWebHandler
• Report Server has its own mechanism, see ThirdPartyDependenciesHandler

Note I am already on it. Not sure if able to finalize that work.

Code review 6692a revision 15171. The changes look good.

1. OpenJDK or Oracle?
2. What is the default to be set? Java 8 or Java 17? When do we fish or cut-bait on the switch of the default to Java 17?
3. Do we need Docker images based off both, or do we make a wholesale changeover?
4. Are script that were changed to handle Java 11 forward compatible with Java 17 (provided they determine Java 11 or greater)? Specifically the SPI configuration is the only thing that really was different in the move from Java 8 to Java 11 that I added to server/client scripts.
5. Do we move customers (mostly I care about my customer :-) to convert/run Java 17 ASAP?

OpenJDK or Oracle?

In the code we should not have dependencies on either one.

In practive, we tend to use OpenJDK because it is good and has permissive licensing.

What is the default to be set? Java 8 or Java 17?

Java 8

When do we fish or cut-bait on the switch of the default to Java 17?

Not yet. We need to look into the performance differences and see if we can eliminate those. If so, then we will gladly move to Java 17 by default.

Do we need Docker images based off both

Yes

or do we make a wholesale changeover?

No

Are script that were changed to handle Java 11 forward compatible with Java 17 (provided they determine Java 11 or greater)? Specifically the SPI configuration is the only thing that really was different in the move from Java 8 to Java 11 that I added to server/client scripts.

This one is for Tomasz.

Do we move customers (mostly I care about my customer :-) to convert/run Java 17 ASAP?

No, for now we let customers decide when to move.

Greg Shah wrote:

OpenJDK or Oracle?

In the code we should not have dependencies on either one.

In practice, we tend to use OpenJDK because it is good and has permissive licensing.

This was more for the standard desktop and other Ansible-scripted environments I am responsible for. Currently I use openjdk-8, so I'll stick with OpenJDK for Java 17.

Greg Shah wrote:

Do we need Docker images based off both

Yes

or do we make a wholesale changeover?

No

I believe I can include both in the base image. But when it comes to the FWD image, I can specify some sort of option in the build of the Docker image such that the default JVM can be specified on the command line.

Follow-up question... if FWD is built with a specific Java, is it forever connected to conversion/runtime of an application using the same version? Or can FWD built with Java 17 be used in a conversion that is done with Java 8 (and vice-versa)? If this is not the case, perhaps I could include both built FWDs in the FWD Docker image, and allow the container creation to choose the one that would be used.

Roger Borrello wrote:

I have some basic questions, since this task is moving steadily towards implementation. Great job! These come from my Docker, standard laptop (#6894) and CI/CD server (#7183) build needs, as well as concern for our customer application timing. Maybe these same questions where answered with the #5567 task, but we'll need this information gathered regardless.

1. OpenJDK or Oracle?

I am on OpenJDK and have no recent experience with Oracle, I guess it depends on customer requirements.

1. Do we need Docker images based off both, or do we make a wholesale changeover?

I work with separate images for each supported java version if you remember thats why I needed suffix option when building docker images:

base_mariadb_11                     jdk11      5dc41c065153   2 months ago   1.07GB
base_mariadb_11                     jdk17      5dc41c065153   2 months ago   1.07GB
base_mariadb_11                     jdk8       5dc41c065153   2 months ago   1.07GB
base_postgres_14                    jdk11      e7182121e72c   2 months ago   935MB
base_postgres_14                    jdk17      e7182121e72c   2 months ago   935MB
base_postgres_14                    jdk8       e7182121e72c   2 months ago   935MB

1. Are script that were changed to handle Java 11 forward compatible with Java 17 (provided they determine Java 11 or greater)? Specifically the SPI configuration is the only thing that really was different in the move from Java 8 to Java 11 that I added to server/client scripts.

Yes they are, in addition in few cases (mostly GWT related) I needed to open module API with --add-opens, You can see sample changes #8208-6 (the change for logging is not needed anymore).

Follow-up question... if FWD is built with a specific Java, is it forever connected to conversion/runtime of an application using the same version? Or can FWD built with Java 17 be used in a conversion that is done with Java 8 (and vice-versa)? If this is not the case, perhaps I could include both built FWDs in the FWD Docker image, and allow the container creation to choose the one that would be used.

I guess its better to stick to the same rule as I proposed for Java 11 - use a single Java version for full building process, despite it would be possible to run Java 8 compiled code on Java 17 (but not vice-versa) or convert with Java 8 and compile with Java 17, there is no difference in generated code (for now).

Tomasz Domin wrote:

I work with separate images for each supported java version if you remember thats why I needed suffix option when building docker images:

base_mariadb_11                     jdk11      5dc41c065153   2 months ago   1.07GB
base_mariadb_11                     jdk17      5dc41c065153   2 months ago   1.07GB
base_mariadb_11                     jdk8       5dc41c065153   2 months ago   1.07GB
base_postgres_14                    jdk11      e7182121e72c   2 months ago   935MB
base_postgres_14                    jdk17      e7182121e72c   2 months ago   935MB
base_postgres_14                    jdk8       e7182121e72c   2 months ago   935MB

This looks odd to me, Tomasz. The base_mariadb_11 images have the same Image ID of 5dc41c065153, which indicates they are the same image, just with different tags. Am I missing something?

Roger Borrello wrote:

Tomasz Domin wrote:

I work with separate images for each supported java version if you remember thats why I needed suffix option when building docker images:

[...]

This looks odd to me, Tomasz. The base_mariadb_11 images have the same Image ID of 5dc41c065153, which indicates they are the same image, just with different tags. Am I missing something?

You are right and I chose wrong examples - I guess there is no java in that images :). Images with java have different signatures:

base_ubuntu_22.04                   jdk11      e19c77610df2   2 months ago   1.56GB
base_ubuntu_22.04                   jdk17      39c29daa7d55   2 months ago   1.6GB
base_ubuntu_22.04                   jdk8       2900252d14f3   2 months ago   1.5GB
base_ubuntu_22.04_pg14              jdk11      038c9d7a1813   2 months ago   2.38GB
base_ubuntu_22.04_pg14              jdk17      b420158e6422   2 months ago   2.42GB
base_ubuntu_22.04_pg14              jdk8       c15a2d245bba   2 months ago   2.32GB
xxxxx_ubuntu_xfer                   14         76c14511151e   2 months ago   53.1GB
xxxxx_ubuntu_xfer                   14_jdk11   aaa7321e868d   2 months ago   53.1GB
xxxxx_ubuntu_xfer                   14_jdk17   2e1117489357   2 months ago   53.2GB
xxxxx_ubuntu_xfer                   14_jdk8    76c14511151e   2 months ago   53.1GB

Code Review Task Branch 6692a Revision 15171

Although I do like managing the dependencies explicitly in our JS code, I don't think we should be loading all of the embedded dependencies as part of fwd_sdk.js. The tabulator and d3 are definitely things that are only used in the analytics (the report server). I don't recall what moment is.

We should only load the minimum dependencies based on the mode (report server, virtual desktop or embedded). In other words, if something is not used in a specific mode then we should not be loading it. Doing that reduces the security exposure (less libraries loaded equals less possible vulnverabilities exposed). It also makes it less likely that other libraries loaded will have conflicts. We should expect customers to be loading their own libraries, especially in embedded mode.

Galya: Please review 6692a revision 15171.

Greg Shah wrote:

Galya: Please review 6692a revision 15171.

fwd_sdk.js is not the right place to load the libraries, it is a lightweight interface for communication with the server on the login page, while these libraries are used after login. Also they are used only with embedded and even then it's not guaranteed. Check hotel_gui where web_partial_login_embedded.html is overwritten in webres with the custom embedded implementation. So it's safe to update the dependencies in gradle and the local paths in EmbeddedWebHandler as it has been done, but no changes to the front-end are needed, since the url paths are not modified. The changes in fwd_sdk.js should be reverted and the relevant part of the comment in EmbeddedWebHandler too. Also, I've noticed the warnings in build.gradle should be updated to state it's EmbeddedWebHandler, not WebResourceHandler.

I agree with the comments from Greg and Galya. Still it makes sense to put the dependencies that are integral part of FWD (surely virtual mode, report server, maybe some dependencies for embedded mode) into a single js to be easily included in the stock welcome pages and in the specific projects. That is a single js for every mode.

Hynek Cihlar wrote:

I agree with the comments from Greg and Galya. Still it makes sense to put the dependencies that are integral part of FWD (surely virtual mode, report server, maybe some dependencies for embedded mode) into a single js to be easily included in the stock welcome pages and in the specific projects. That is a single js for every mode.

What is the benefit?

The libraries in concern in r15171 are used only by custom front-ends in the embedded mode.

Galya B wrote:

Hynek Cihlar wrote:

I agree with the comments from Greg and Galya. Still it makes sense to put the dependencies that are integral part of FWD (surely virtual mode, report server, maybe some dependencies for embedded mode) into a single js to be easily included in the stock welcome pages and in the specific projects. That is a single js for every mode.

What is the benefit?

The libraries in concern in r15171 are used only by custom front-ends in the embedded mode.

There are specific dependencies needed for FWD to run in embedded mode. If they are included in a single bundle which will be managed inside FWD itself, it will simplify deployment/upgrades.

The need for an upgrade will come from FWD product upgrade, but also from the vulnerability check, which may happen often.

Hynek Cihlar wrote:

Galya B wrote:

Hynek Cihlar wrote:

I agree with the comments from Greg and Galya. Still it makes sense to put the dependencies that are integral part of FWD (surely virtual mode, report server, maybe some dependencies for embedded mode) into a single js to be easily included in the stock welcome pages and in the specific projects. That is a single js for every mode.

What is the benefit?

The libraries in concern in r15171 are used only by custom front-ends in the embedded mode.

There are specific dependencies needed for FWD to run in embedded mode. If they are included in a single bundle which will be managed inside FWD itself, it will simplify deployment/upgrades.

The need for an upgrade will come from FWD product upgrade, but also from the vulnerability check, which may happen often.

I think this is not related to the current upgrade to java 17. Do you mind to create a new task for further discussion?

Galya B wrote:

I think this is not related to the current upgrade to java 17. Do you mind to create a new task for further discussion?

Makes sense, see #8651.

Greg Shah wrote:

Code Review Task Branch 6692a Revision 15171

Although I do like managing the dependencies explicitly in our JS code, I don't think we should be loading all of the embedded dependencies as part of fwd_sdk.js. The tabulator and d3 are definitely things that are only used in the analytics (the report server). I don't recall what moment is.

In build.gradle dependencies they are all grouped under fwdClient group, should they be split into subgroups for clarity ?

We should only load the minimum dependencies based on the mode (report server, virtual desktop or embedded). In other words, if something is not used in a specific mode then we should not be loading it. Doing that reduces the security exposure (less libraries loaded equals less possible vulnverabilities exposed). It also makes it less likely that other libraries loaded will have conflicts. We should expect customers to be loading their own libraries, especially in embedded mode.

I've checked and virtual desktop does not need any of these libraries, they are specific to embedded mode (as stated below). Not sure what reporting server needs.

Galya B wrote:

Greg Shah wrote:

Galya: Please review 6692a revision 15171.

...So it's safe to update the dependencies in gradle and the local paths in @EmbeddedWebHandler as it has been done, but no changes to the front-end are needed, since the url paths are not modified.

Its not true for all the cases - url path are modified - see jquery, and may be further modified in future. The way they are modified depends on the way dependencies are managed - e.g. WebJars or other way, URL mapping in EmbeddedWebHandler only works for path prefix.

Also, I've noticed the warnings in build.gradle should be updated to state it's EmbeddedWebHandler, not WebResourceHandler.

Nice catch, I'll update it.

Galya B wrote:

Hynek Cihlar wrote:

I agree with the comments from Greg and Galya. Still it makes sense to put the dependencies that are integral part of FWD (surely virtual mode, report server, maybe some dependencies for embedded mode) into a single js to be easily included in the stock welcome pages and in the specific projects. That is a single js for every mode.

What is the benefit?

The libraries in concern in r15171 are used only by custom front-ends in the embedded mode.

I agree with Hynek regarding benefits, there are JS script we do care about in FWD and lets state it clear by putting them into a dedicated file.
It should also make updating/upgrading dependencies easier - there are two files to be changed only, no need to dig into customer embedded mode implementations.

I also agree that maybe instead putting embedded mode dependencies into fwd_sdk.js we should put it into another file, maybe like fwd_embedded.js ?

Tomasz Domin wrote:

Galya B wrote:

Greg Shah wrote:

Galya: Please review 6692a revision 15171.

...So it's safe to update the dependencies in gradle and the local paths in @EmbeddedWebHandler as it has been done, but no changes to the front-end are needed, since the url paths are not modified.

Its not true for all the cases - url path are modified - see jquery, and may be further modified in future. The way they are modified depends on the way dependencies are managed - e.g. WebJars or other way, URL mapping in EmbeddedWebHandler only works for path prefix.

The third argument in the WebResourceHandler constructor is not part of the url, it's used for server-side mapping of the url context path to the local resource path. That being said, there is still a flaw in the design of this API, because the resources are supposed to be used by custom customer code and upgrading the versions and/or paths will break their dependencies. EmbeddedWebHandler maps a whole lib dir to a context path, but the url paths accessing specific lib files will have to be changed in all customer front-ends.

So the question is do we expose these libraries only for hotel_gui embedded example or we want to allow the customers to build their pages upon them. If the first is true, then you just need to update hotel_gui/webres/web_partial_login_embedded.html as well. If the second is true, we need to come up with a better API and the changes to the updates of the js libs can be reverted for now.

I also agree that maybe instead putting embedded mode dependencies into fwd_sdk.js we should put it into another file, maybe like fwd_embedded.js ?

Let's discuss it further in #8651.

Tomasz Domin wrote:

Galya B wrote:

The libraries in concern in r15171 are used only by custom front-ends in the embedded mode.

I agree with Hynek regarding benefits, there are JS script we do care about in FWD and lets state it clear by putting them into a dedicated file.
It should also make updating/upgrading dependencies easier - there are two files to be changed only, no need to dig into customer embedded mode implementations.

I also agree that maybe instead putting embedded mode dependencies into fwd_sdk.js we should put it into another file, maybe like fwd_embedded.js ?

Actually these js dependencies should be removed from the server build. They should be added by the customer project, if used in their embedded page. There is already a mechanism for adding paths for custom web resources in the customer projects. This way FWD will not be responsible for upgrading / breaking customer front-end code. We'll update hotel_gui by adding the libs in the webres dir and update the src urls in web_partial_login_embedded.html. We can do it this way for hotel_gui, but in general I would recommend customers to include dependencies from the original lib urls for better cacheability or use their own package managers and minifier pipelines before distributing by adding in webres.

Greg, Constantin, what do you think about removing the js files for the custom embedded pages from the server dependencies? I don't see a reason for them to be always packed with FWD.

• org.slf4j:jcl-over-slf4j:1.7.36 is replaced by org.slf4j:slf4j-jdk14:2.0.12. FWD has custom slf4j implementation, so we must not have slf4j-jdk14 or similar in our dependencies. Also jcl-over-slf4j is needed, it's a bridge binding other APIs to slf4j.
• As for commons-logging:commons-logging: it is a facade for logging, alternative to jcl-over-slf4j, so it moves 3rd party logging away from our slf4j implementation. If its found in the transitive dependencies, it should be added to:
  

  if (JavaVersion.current().isCompatibleWith(JavaVersion.VERSION_11)) {
         exclude group: 'commons-logging', module: "commons-logging" 
  

• org.jboss.logging:jboss-logging:3.5.3.Final is added, but I don't see where it's used.
• As discussed in #8555 org.apache.logging.log4j:log4j dependencies should be removed.

• src/com/goldencode/p2j/report/web/res/index.html (the 3pl context is removed in ReportWebServer.ThirdPartyDependenciesHandler when mapping to res files);
• After merge, hotel_gui/webres/web_partial_login_embedded.html;
• After merge, any customer code using web embedded. We probably need to find another long-term solution for the dependencies in hotel_gui and customer embedded pages, but we can do so in #8651.

The logging patch looks reasonable. Tomasz: do you have any objections? If not, this should be committed into 6692a.

will be added to server.sh / client startup scripts to default to com.goldencode.p2j.util.logging.CentralSlf4jProvider or to another string, if the customer has a different impl

Galya: Do you have a suggested patch for the Hotel GUI project?

Greg Shah wrote:

The logging patch looks reasonable. Tomasz: do you have any objections? If not, this should be committed into 6692a.

No, I don't, changes to logging were needed and Galya helped me a lot with her logging patch.
I am in a process of rebasing to latest trunk and committing the changes

Attached a patch to hotel_gui to make slf4j2 logging work (basically following property -Dslf4j.provider=com.goldencode.p2j.util.logging.CentralSlf4jProvider needs to be added to a server command line)

Is our default safe? In other words, can we run without setting anything on the command line? I would hope that setting that value would only be needed to override the default.

Greg Shah wrote:

Attached a patch to hotel_gui to make slf4j2 logging work (basically following property -Dslf4j.provider=com.goldencode.p2j.util.logging.CentralSlf4jProvider needs to be added to a server command line)

Is our default safe? In other words, can we run without setting anything on the command line? I would hope that setting that value would only be needed to override the default.

From https://www.slf4j.org/faq.html#changesInVersion200

In SLF4J 2.0.x and later, during its initalization, LoggerFactory looks up SLF4JServiceProvider instances using the Java platfom's ServiceLoader mechanism. An artifact containing an implementation of the SLF4JServiceProvider interface is called a "provider".

since 2.0.9 You can specify the provider class explicitly via the "slf4j.provider" system property. This bypasses the service loader mechanism for finding providers and may shorten SLF4J initialization.

With slf4j api v2 there are two methods of loading the logging provider: The default one with ServiceLoader should be automatic, but I find in example implementations that the custom package containing the SLF4JServiceProvider implementation has module-info.java where:

module custom.package { 
  requires org.slf4j;
  requires java.logging;
  provides org.slf4j.spi.SLF4JServiceProvider with custom.package.CentralSlf4jProvider;

  exports org.slf4j.jul;
  opens custom.package to org.slf4j;
}

P.S. When slf4j impl is found there are a few lines in server logs indicating it has been loaded successfully:

INFO | com.mchange.v2.log.MLog | ThreadName:MLog-Init-Reporter | MLog clients using slf4j logging.

INFO | org.eclipse.jetty.util.log | ThreadName:main, Session:00000000, Thread:00000001, User:standard | Logging initialized @6054ms to org.eclipse.jetty.util.log.Slf4jLog

Tomasz Domin wrote:

I've added CentralSlf4jProvider as service and it seems changes to server.sh are not needed anymore.
Attached a patch CentralSlf4jProvider.diff to enable a service without a need for a command line parameter - it works fine for hotel_gui.

I see how this works and it can give us a default, but now the issue is that some customers need to disable our slf4j implementation and I thought it will be easier with the property. Although #7589 is not likely to happen with slf4j api v2, we still need to allow the customers to plug in their own logging. With the jar approach I'm not sure if the service provider can be changed or disabled in customer projects having p2j.jar in the classpath.

Galya - I have a question on slf4j bridges - why only jcl-over-slf4j is included ? I guess log4j is eliminated so we dont need log4j-over-slf4j, but what about jul-to-slf4j ?

When I originally migrated logging to the CentralLogger from stderr (where the java.util.logging writes also went) I haven't found missing logs or a library complaining about loggers. If we can pinpoint a library that is important to log, but the logs are missing, then we can consider some additional bridges. Otherwise, we should not add more logging dependencies interfering with the customer projects. Also jul-to-slf4j seems to have major performance issues, when not configured well, more at https://www.slf4j.org/legacy.html.

Tomasz, can you test if the property overwrites the service loader? If it works, it should be fine.

Also, to make sure the patch for the service loader works, make sure to find the two messages from the bottom of #6692-108 in the server logs.

Galya B wrote:

Tomasz, can you test if the property overwrites the service loader? If it works, it should be fine.

Yes, in case slf4j.provider is defined service loader does not even start - SLF4J simply loads specified provider e.g.
For -Dslf4j.provider=com.goldencode.p2j.util.logging.CentralSlf4jProvider

SLF4J(I): Attempting to load provider "com.goldencode.p2j.util.logging.CentralSlf4jProvider" specified via "slf4j.provider" system property

SLF4J(I): Attempting to load provider "org.slf4j.simple.SimpleServiceProvider" specified via "slf4j.provider" system property

Galya B wrote:

Also, to make sure the patch for the service loader works, make sure to find the two messages from the bottom of #6692-108 in the server logs.

Yes, they are there:

tjd/1.8.0_402:~/projects/hotel_gui_6692a$ grep -e MLog -e "Logging initialized" deploy/logs/*
deploy/logs/fwd_server_20240423_161807_0_0.log:24/04/23 16:18:09.436+0200 |    INFO | com.mchange.v2.log.MLog | ThreadName:MLog-Init-Reporter | MLog clients using slf4j logging.
deploy/logs/fwd_server_20240423_161807_0_0.log:24/04/23 16:18:11.902+0200 |    INFO | org.eclipse.jetty.util.log | ThreadName:main, Session:00000000, Thread:00000001, User:standard | Logging initialized @4701ms to org.eclipse.jetty.util.log.Slf4jLog

OK, just one last clarification: Can you override the service loader with an empty property -Dslf4j.provider=?

Galya B wrote:

OK, just one last clarification: Can you override the service loader with an empty property -Dslf4j.provider=?

Actually this is likely not going to be needed.

I'm ok with the logging changes. I think they can be applied, if not on the branch yet.

Galya B wrote:

OK, just one last clarification: Can you override the service loader with an empty property -Dslf4j.provider=?

No, it cant.
But I found a way for disabling any service provider when service loader tries to discover it - it is needed throw an RuntimeException in its constructor :) like:

   public CentralSlf4jProvider ()
   {
      if ("true".equals( System.getProperty("disableSLF4JServiceProvider")))
      {
         throw new RuntimeException();
      }
   }

In that case we get: org.slf4j.spi.SLF4JServiceProvider: Provider com.goldencode.p2j.util.logging.CentralSlf4jProvider could not be instantiated and SLF tries to load the next one.

It could be an option if needed later on. For now I think it's not necessary. But I remembered a few more places that need update. The default "fwd-slf4j" in ClientBuilderOptions should be null, fwd-slf4j.jar should be removed from postbuild.sh, import.sh. ./slf4j-impl.jar should be removed from the options in spawn.c and winspawn.c. These are not critical, just cleanup.

Galya B wrote:

But I remembered a few more places that need update. The default "fwd-slf4j" in ClientBuilderOptions should be null, fwd-slf4j.jar should be removed from postbuild.sh, import.sh. ./slf4j-impl.jar should be removed from the options in spawn.c and winspawn.c. These are not critical, just cleanup.

Greg, should I update postbuild.sh and import.sh in this branch?

Also, a question, since I may have missed a detail... are there changes to be made in the client.sh and/or server.sh files to set options appropriately as defaults? Note that I had made changes to install_spawner.sh when we needed to include fwd-slf4j.jar, so we may need to make changes to match this branch when that time comes.

Roger Borrello wrote:

Also, a question, since I may have missed a detail... are there changes to be made in the client.sh and/or server.sh files to set options appropriately as defaults? Note that I had made changes to install_spawner.sh when we needed to include fwd-slf4j.jar, so we may need to make changes to match this branch when that time comes.

We'll have to revert to cleanup, because the new mechanism of loading will have the default as part of p2j.jar. Only clients who don't need our implementation will need to add a property to client.sh and/or server.sh to change the slf4j provider. I think the customer you're asking about will need simply a revert when time comes.

Galya B wrote:

We'll have to revert to cleanup, because the new mechanism of loading will have the default as part of p2j.jar. Only clients who don't need our implementation will need to add a property to client.sh and/or server.sh to change the slf4j provider. I think the customer you're asking about will need simply a revert when time comes.

Thanks for the details!

Also, a question, since I may have missed a detail... are there changes to be made in the client.sh and/or server.sh files to set options appropriately as defaults?

I'm OK with providing an option to override the default. If the option is not specified, then no property should be set (so that the default will naturally be used).

This way customers can use the standard version of our scripts.

Greg Shah wrote:

Also, a question, since I may have missed a detail... are there changes to be made in the client.sh and/or server.sh files to set options appropriately as defaults?

I'm OK with providing an option to override the default. If the option is not specified, then no property should be set (so that the default will naturally be used).

This way customers can use the standard version of our scripts.

Default setting will use CentralSlf4jProvider, I've added an option to block it by setting disableCentralSlf4jProvider system property. In that case an alternative slf4j provider needs to be added to the JVM classpath.
Pushed up to revision 15183.