Hi Simon,
On 04/19/2017 11:25 AM, Doug Simon wrote:
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).
Looks good.
I was not aware of the new Map.ofEntries method. Nice to see more space
efficient map implementations being added to the JDK.
Admittedly, I used this method in a way not envisioned by the author.
Maybe there's a reason there is no Map.copyOf(Map) method there, which
would make this even simpler. If there was one, it would be too easy to
(mis)use it instead of Collections.unmodifiableMap(Map), albeit with a
slightly different semantics, and force re-hashing-copying of big maps
where there is no need to do that. But it would be a pretty nice
replacement for the following idiom:
Collections.unmodifiableMap(new HashMap<>(someMap))
Regards, Peter
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
