eclipse-ee4j / eclipselink

Eclipselink project
https://eclipse.dev/eclipselink/
Other
198 stars 168 forks source link

Valhalla Support #1047

Open jgrassel opened 3 years ago

jgrassel commented 3 years ago

I've been made aware that Valhalla, an upcoming technology to be added to Java, is going to introduce language breaking changes (https://openjdk.java.net/jeps/390 and https://openjdk.java.net/jeps/169).

One example in Eclipselink code is at https://github.com/eclipse-ee4j/eclipselink/blob/2.7/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/databaseaccess/DatasourcePlatform.java#L780

    /** Ensures that only one thread at a time can add/remove sequences */
    protected Object sequencesLock = new Boolean(true);
...
    public void addSequence(Sequence sequence, boolean isSessionConnected) {
        synchronized(sequencesLock) {

One of Valhalla's changes is to the Primitive wrapper types, it's going to introduce a memory optimization which will make them value types. This means that new Boolean(true) will always return the same object, instead of creating a new object instance of Boolean with a value of true. This means the above synchronized block are all going to key on the same object, instead of the object constructed for that instance of DatasourcePlatform.

The above is one example, the source will have to be combed for synchronized uses of other Primitive wrappers before Valhalla is released into a future JDK.

dazey3 commented 3 years ago

It appears that org.eclipse.persistence.internal.jpa.deployment.JavaSECMPInitializer.getJavaSECMPInitializer also has this issue on 2.6_WAS: https://github.com/eclipse-ee4j/eclipselink/blob/2.6_WAS/jpa/org.eclipse.persistence.jpa/src/org/eclipse/persistence/internal/jpa/deployment/JavaSECMPInitializer.java#L61

For 2.7+, it looks like @lukasj fixed the issue in this commit: https://github.com/eclipse-ee4j/eclipselink/commit/05ffcf76bab398c5360f2789203a34b70eb46791

jgrassel commented 3 years ago

Reading https://openjdk.java.net/jeps/390 it sounds like we have three things to watch out for (as far as I've read so far):

  1. Instances of these classes that are equal (per equals) may also be considered identical (per ==), potentially breaking programs that rely on a != result for correct behavior.
  2. Attempts to create wrapper class instances with new Integer, new Double, etc., rather than implicit boxing or calls to the valueOf factory methods, will produce LinkageErrors.
  3. Attempts to synchronize on instances of these classes will produce exceptions.
AleksNo commented 3 years ago

Hi, the constructors of the wrapper types are now deprecated for removal. Code which uses those constructors will not compile anymore when they remove them.

https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/lang/Boolean.html

Cheers

lukasj commented 3 years ago

not sure reliance on default constructors falls to this but we should definitely address it (see JDK-8250212 for details)