RFR: 8177845: Need a mechanism to load Graal

[Peter Levart]

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.levart at 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
>>