Hi Peter, All of your suggestions look good. I've updated http://cr.openjdk.java.net/~dnsimon/8177845/jdk/src/java.base/share/classes/jdk/internal/misc/VM.java.udiff.html to include them (please check I didn't make any copy errors in the process).
I was not aware of the new Map.ofEntries method. Nice to see more space efficient map implementations being added to the JDK. Thanks! -Doug > On 19 Apr 2017, at 10:12, Peter Levart <peter.lev...@gmail.com> wrote: > > > > On 04/19/2017 09:42 AM, Peter Levart wrote: >> >> >> On 04/19/2017 09:37 AM, Peter Levart wrote: >>> >>> >>> http://cr.openjdk.java.net/~plevart/jdk9-dev/8177845_VM.getSavedProperties/webrev.01/ >>> >> >> Also, while we are at it, the following javadocs in the getSavedProperty() >> do not apply any more: >> >> 138 * It accesses a private copy of the system properties so >> 139 * that user's locking of the system properties object will not >> 140 * cause the library to deadlock. >> >> In JDK 9, Properties class does not use locking any more on the Properties >> instance for get()/getProperty() methods... >> >> Regards, Peter >> > > I also noticed the following comment: > > // TODO: the Property Management needs to be refactored and > // the appropriate prop keys need to be accessible to the > // calling classes to avoid duplication of keys. > private static Map<String, String> savedProps; > > ...which is not entirely true. Neither keys nor values are duplicated (they > are just referenced in the new copy of the Properties/Map object). What is > duplicated is an excessive amount of internal objects, such as array slots > and Map.Entry objects. If this is a concern, then we could use the new > immutable Map implementation that is available in JDK 9, so the following > lines in my webrev: > > 181 @SuppressWarnings("unchecked") > 182 Map<String, String> sp = new HashMap<>((Map)props); > 183 // only main thread is running at this time, so savedProps and > 184 // its content will be correctly published to threads started > later > 185 savedProps = Collections.unmodifiableMap(sp); > > Could be changed into: > > @SuppressWarnings("unchecked") > Map<String, String> sp = > Map.ofEntries(props.entrySet().toArray(new Map.Entry[0])); > // only main thread is running at this time, so savedProps > // will be correctly published to threads started later > savedProps = sp; > > ...to save some excessive space (the implementation is a linear-probe > hashtable which keeps keys and values directly in an array without wrapping > them with Map.Entry objects). > > Regards, Peter >
