On Thu, 9 Feb 2023 15:05:13 GMT, Per Minborg <pminb...@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
