Review Request: JDK-8193159: Reduce the number of classes loaded due to NativeLibrary
A tiny startup fix - useArrayDeque instead of LinkedList for ClassLoader.NativeLibrary which is typically loaded at startup for example when loading a JAR file. Thanks Mandy diff --git a/src/java.base/share/classes/java/lang/ClassLoader.java b/src/java.base/share/classes/java/lang/ClassLoader.java --- a/src/java.base/share/classes/java/lang/ClassLoader.java +++ b/src/java.base/share/classes/java/lang/ClassLoader.java @@ -38,6 +38,7 @@ import java.security.PrivilegedAction; import java.security.ProtectionDomain; import java.security.cert.Certificate; +import java.util.ArrayDeque; import java.util.Arrays; import java.util.Collections; import java.util.Deque; @@ -45,7 +46,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Hashtable; -import java.util.LinkedList; import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; @@ -2496,7 +2496,7 @@ } // native libraries being loaded - static Deque<NativeLibrary> nativeLibraryContext = new LinkedList<>(); + static Deque<NativeLibrary> nativeLibraryContext = new ArrayDeque<>(); /* * The run() method will be invoked when this class loader becomes
Google decided that LinkedList is almost never the best choice, so any use of LinkedList is discouraged. ArrayDeque's backing array never shrinks, so you might want to give it an explicit initial size. On Wed, Dec 6, 2017 at 4:33 PM, mandy chung <mandy.chung@oracle.com> wrote:
A tiny startup fix - useArrayDeque instead of LinkedList for ClassLoader.NativeLibrary which is typically loaded at startup for example when loading a JAR file.
Thanks Mandy
diff --git a/src/java.base/share/classes/java/lang/ClassLoader.java b/src/java.base/share/classes/java/lang/ClassLoader.java --- a/src/java.base/share/classes/java/lang/ClassLoader.java +++ b/src/java.base/share/classes/java/lang/ClassLoader.java @@ -38,6 +38,7 @@ import java.security.PrivilegedAction; import java.security.ProtectionDomain; import java.security.cert.Certificate; +import java.util.ArrayDeque; import java.util.Arrays; import java.util.Collections; import java.util.Deque; @@ -45,7 +46,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Hashtable; -import java.util.LinkedList; import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; @@ -2496,7 +2496,7 @@ }
// native libraries being loaded - static Deque<NativeLibrary> nativeLibraryContext = new LinkedList<>(); + static Deque<NativeLibrary> nativeLibraryContext = new ArrayDeque<>();
/* * The run() method will be invoked when this class loader becomes
On 12/6/17 6:08 PM, Martin Buchholz wrote:
Google decided that LinkedList is almost never the best choice, so any use of LinkedList is discouraged.
ArrayDeque's backing array never shrinks, so you might want to give it an explicit initial size.
The initial size is 16 which is okay. I can make it smaller instead: - static Deque<NativeLibrary> nativeLibraryContext = new LinkedList<>(); + static Deque<NativeLibrary> nativeLibraryContext = new ArrayDeque<>(8); Mandy
On Wed, Dec 6, 2017 at 4:33 PM, mandy chung <mandy.chung@oracle.com <mailto:mandy.chung@oracle.com>> wrote:
A tiny startup fix - useArrayDeque instead of LinkedList for ClassLoader.NativeLibrary which is typically loaded at startup for example when loading a JAR file.
Thanks Mandy
diff --git a/src/java.base/share/classes/java/lang/ClassLoader.java b/src/java.base/share/classes/java/lang/ClassLoader.java --- a/src/java.base/share/classes/java/lang/ClassLoader.java +++ b/src/java.base/share/classes/java/lang/ClassLoader.java @@ -38,6 +38,7 @@ import java.security.PrivilegedAction; import java.security.ProtectionDomain; import java.security.cert.Certificate; +import java.util.ArrayDeque; import java.util.Arrays; import java.util.Collections; import java.util.Deque; @@ -45,7 +46,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Hashtable; -import java.util.LinkedList; import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; @@ -2496,7 +2496,7 @@ }
// native libraries being loaded - static Deque<NativeLibrary> nativeLibraryContext = new LinkedList<>(); + static Deque<NativeLibrary> nativeLibraryContext = new ArrayDeque<>();
/* * The run() method will be invoked when this class loader becomes
On 07/12/2017 05:04, mandy chung wrote:
On 12/6/17 6:08 PM, Martin Buchholz wrote:
Google decided that LinkedList is almost never the best choice, so any use of LinkedList is discouraged.
ArrayDeque's backing array never shrinks, so you might want to give it an explicit initial size.
The initial size is 16 which is okay. I can make it smaller instead:
- static Deque<NativeLibrary> nativeLibraryContext = new LinkedList<>(); + static Deque<NativeLibrary> nativeLibraryContext = new ArrayDeque<>(8);
This looks okay. It seems unlikely that hundreds of native libraries will be loaded, then unloaded, and the backing array being a memory concern. -Alan
On 2017-12-07 14:39, Alan Bateman wrote:
On 07/12/2017 05:04, mandy chung wrote:
On 12/6/17 6:08 PM, Martin Buchholz wrote:
Google decided that LinkedList is almost never the best choice, so any use of LinkedList is discouraged.
ArrayDeque's backing array never shrinks, so you might want to give it an explicit initial size.
The initial size is 16 which is okay. I can make it smaller instead:
- static Deque<NativeLibrary> nativeLibraryContext = new LinkedList<>(); + static Deque<NativeLibrary> nativeLibraryContext = new ArrayDeque<>(8);
+1
This looks okay. It seems unlikely that hundreds of native libraries will be loaded, then unloaded, and the backing array being a memory concern.
It also needs to load said hundreds of native libraries in parallel for this context queue to grow in the first place, right? So I'm not concerned, but on that tangent I have been wondering for some time why ArrayDeque doesn't have a trimToSize method like ArrayList does. Theoretically it'd be nice to have some means to allow periodically taking a look at persistent queues like this one and trim the backing arrays down if they ever outgrow their purpose. I guess it's just never become a real problem... /Claes
On 12/7/17 5:39 AM, Alan Bateman wrote:
On 07/12/2017 05:04, mandy chung wrote:
On 12/6/17 6:08 PM, Martin Buchholz wrote:
Google decided that LinkedList is almost never the best choice, so any use of LinkedList is discouraged.
ArrayDeque's backing array never shrinks, so you might want to give it an explicit initial size.
The initial size is 16 which is okay. I can make it smaller instead:
- static Deque<NativeLibrary> nativeLibraryContext = new LinkedList<>(); + static Deque<NativeLibrary> nativeLibraryContext = new ArrayDeque<>(8);
This looks okay. It seems unlikely that hundreds of native libraries will be loaded, then unloaded, and the backing array being a memory concern.
Yup, it's not a big concern. I believe the number of elements in this deque is typically very small number. This deque is used to keep the native libraries are being loaded and its context used by JNI FindClass when called from JNI_OnLoad. Mandy
+1
participants (4)
-
Alan Bateman
-
Claes Redestad
-
mandy chung
-
Martin Buchholz