Skip to content

Conversation

bowbahdoe
Copy link

@bowbahdoe bowbahdoe commented Mar 8, 2024

There is only one place in commons-lang where anything from java.desktop is used, and that is AbstractCircuitBreaker.

This changes that code to lazily initialize its internal PropertyChangeSupport. This makes it so that it is safe to initialize and use that class without java.desktop on the module-path.

Upsides:

  • Smaller jlinked images for every apache commons library
  • No binary compatibility changes

Downsides:

  • Likely a small performance hit
  • It is possible for a user to give a null instance of PropertyChangeListener without depending on the java.desktop module, in which case the behavior of the method will change from "ignores null" to "fails"
  • I do not know how to add a test for this to the junit suite. We'd have to run a subset of the tests without java.desktop present. Currently I am validating manually like so
$ mvn clean compile package
$ jlink --module-path target/commons-lang3-3.15.0-SNAPSHOT.jar --add-modules org.apache.commons.lang3,jdk.jshell --output jre
$ ./jre/bin/java --list-modules
java.base@21.0.1
java.compiler@21.0.1
java.logging@21.0.1
java.prefs@21.0.1
java.xml@21.0.1
jdk.attach@21.0.1
jdk.compiler@21.0.1
jdk.internal.ed@21.0.1
jdk.internal.jvmstat@21.0.1
jdk.internal.le@21.0.1
jdk.internal.opt@21.0.1
jdk.internal.vm.ci@21.0.1
jdk.jdi@21.0.1
jdk.jdwp.agent@21.0.1
jdk.jshell@21.0.1
jdk.zipfs@21.0.1
org.apache.commons.lang3@3.15.0-SNAPSHOT

$ ./jre/bin/jshell

jshell> var breaker = new EventCountCircuitBreaker(1000, 1, java.util.concurrent.TimeUnit.SECONDS);
breaker ==> org.apache.commons.lang3.concurrent.EventCountCircuitBreaker@1ce92674

jshell> breaker.addChangeListener(null);
|  Error:
|  cannot access java.beans.PropertyChangeListener
|    class file for java.beans.PropertyChangeListener not found
|  breaker.addChangeListener(null);
|  ^-----------------------------^

Before

$ du -sh jre
125M    jre

$ ./jre/bin/java --list-modules
java.base@21.0.1
java.datatransfer@21.0.1
java.desktop@21.0.1
java.prefs@21.0.1
java.xml@21.0.1
jdk.internal.vm.ci@21.0.1
org.apache.commons.lang3@3.15.0-SNAPSHOT

After

$ du -sh jre
 88M    jre

$ ./jre/bin/java --list-modules
java.base@21.0.1
jdk.internal.vm.ci@21.0.1
org.apache.commons.lang3@3.15.0-SNAPSHOT

@garydgregory
Copy link
Member

-1: This looks too hacky and brittle to me as we do not have ways to prevent future changes from depending on java.desktop or other modules which would make this moot. For 4.0, we can talk about making Lang a java.base only module. TBD.

@garydgregory
Copy link
Member

Closing: For 4.0, we can remove the dependency on java.desktop, probably.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants