Hi Claes,
On 1/20/20 7:54 AM, Claes Redestad wrote:
Hi,
some minor cleanups and enhancements in and around java.lang.ClassLoader
which add up to a small startup improvement:
- Remove use of Vector/Hashtable from ClassLoader, along with a few
other improvements/modernizations.
- Refactor ClassLoader::sys_paths/user_paths so that they're initialized
lazily but also published safely
Webrev: http://cr.openjdk.java.net/~redestad/8236075/open.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8236075
This patch looks okay in general. Below are some small comments:
ClassLoaderHelper.java
Moving ClassLoader::initializePath path-parsing method to
ClassLoaderHelper is okay while duplicating the code in macOS and unix
CLH implementation is less ideal. One option to consider is to add a
utility method in a shared class like parsePath(String paths, boolean
allowQuote).
OTOH, it's also okay to me since this duplicated code is small for now
that we can follow up as part of the native library handling refactoring
I'm working on. But I will suggest to use the same pathStart and
pathEnd variable names as in the windows implementation.
StaticProperty.java
I believe "java.library.path" and "sun.boot.library.path" are
always set to non-null. Since the existing code assumes them that may
be null, I agree with you to keep the existing logic as we discussed.
We will follow up this later.
You added getProperty(Properties props, String key). Do you see the
change to replace System.getProperty calls with one single call to
System.getProperties and Properties.getProperty is significant to
performance? i.e. what if you keep the static method to
getProperty(String key) (was initProperty before your patch) and add
getProperty(String key, String defaultValue) where the defaultValue may
be null.
Mandy
