RFR: 8301578: Perform output outside synchronization in Module.class [v4]

[Alan Bateman]

On Thu, 9 Feb 2023 15:05:13 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> This PR proposed to reduce contention in synchronized methods mainly by doing I/O operations outside synch blocks.
>
> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use Unsafe instead of synchronized

src/java.base/share/classes/java/lang/Module.java line 120:

> 118:     // memory semantics that preserves ordering and visibility across threads.
> 119:     //
> 120:     // Used reflectively via Unsafe

I assume L119-120 can be removed.

src/java.base/share/classes/java/lang/Module.java line 132:

> 130:            ClassLoader loader,
> 131:            ModuleDescriptor descriptor,
> 132:            URI uri) {

This seems a spurious edit, not sure if you meant to change it.

src/java.base/share/classes/java/lang/Module.java line 209:

> 207:      *
> 208:      * @return The class loader for this module
> 209:      * @throws SecurityException If denied by the security manager

The exception messages are aligned under the exceptions in this class so I assume you didn't mean to change this.

src/java.base/share/classes/java/lang/Module.java line 233:

> 231:      * Returns the module layer that contains this module or {@code null} if
> 232:      * this module is not in a module layer.
> 233:      * <p>

This will break up the first paragraph, I don't think this PR should be changing that.

src/java.base/share/classes/java/lang/Module.java line 286:

> 284: 
> 285:         private static final Unsafe UNSAFE = Unsafe.getUnsafe();
> 286:         private static final long FIELD_OFFSET = UNSAFE.objectFieldOffset(Module.class, "enableNativeAccess");

Using Unsafe and CAS'ing the enableNativeAccess field looks okay.  The name "AccessHolder" looks too general here as there is a lot of access going on in this class, maybe NativeAccessHolder will work.

It would be good if you could try to keep changes to this code consistent with the existing style if possible. In this case, the other inner class uses /** .. */ style comment for the class description. I think would I put the constants at the top of the this class.

-------------

PR: https://git.openjdk.org/jdk/pull/12193