Hi Mandy,
On 02.10.2019 01:08, Anton Kozlov wrote:>
> > On 30.09.2019 21:18, Mandy Chung wrote:
>> Skimming through the implementation, it seems to me that the
>> Runtime::loadLibrary0 does not need to be synchronized.
>> ClassLoader::loadLibrary0 should ensure that a native library of a given
>> name is loaded and registered only once.
>
> Interesting. I get confused at first by a comment nearby [1]. At first sight,
> it assumes that Runtime.loadLibrary0 is synchronized. But it's guarded by a
> monitor of loadedLibraryNames.
>
> May be this will be simpler approach.
I cannot find a situation when this will be wrong. All the path from
Runtime.loadLibrary (and Runtime.load) to NativeLibrary.loadLibrary do not need
to be synchronized. Except for ClassLoader.sys_paths and usr_paths static
fields initialization, they should be synchronized.
I looked at older releases, OpenJDK 7, 8. Relevant code is pretty much the
same: Runtime.{load,loadLibrary} are synced [1], ClassLoader.loadLibrary
eventually grabs a lock [2] and do bookkeeping for loading a native library
only once.
So, if the Runtime.loadLibrary synchronization is redundant for JDK14, then
it's so for older jdks, from the very beginning of _Open_JDK. I'd wish someone
commented why is it there. Is it redundant since then.
I've updated the webrev with this approach and changed the comment. Link is at
the bottom.
[1]:
https://hg.openjdk.java.net/jdk6/jdk6/jdk/file/jdk6-b00/src/share/classes/java/lang/Runtime.java#l779
[2]:
https://hg.openjdk.java.net/jdk6/jdk6/jdk/file/jdk6-b00/src/share/classes/java/lang/ClassLoader.java#l1732
On 03.10.2019 01:59, Mandy Chung wrote:
> Just on the test, instead of storing Target and Target2 classfile bytes in
> the source, the test can use jdk.test.lib.compiler.CompilerUtils
Thanks for pointing that out, I've changed the test too.
Updated webrev:
http://cr.openjdk.java.net/~akozlov/8231584/webrev.02/
Thanks,
Anton