RFR(S) 8004872: Early loading of HashMap under -XX:+AggressiveOpts can be removed

[David Holmes]

Hi Calvin,

On 11/07/2013 11:09 AM, Calvin Cheung wrote:
> I've updated the webrev with your suggestion:
>      http://cr.openjdk.java.net/~ccheung/8004872/webrev/
>
> StringValue isn't in jdk7 either.
> Based on 6695005 ((alt-rt, str) Add caching of String values), it was
> fixed in 6u6p and has a 7-na keyword.

More digging needed here ...

StringValue may not be in 7 but UseStringCache is/was. 6714404 added the 
code you are deleting but that version of it didn't pre-load StringValue 
just String:

http://hg.openjdk.java.net/jdk7/hotspot/hotspot/rev/6d13fcb3663f

That was 7-b29. Then 6810653 made the change from String to StringValue 
in 7-b53.

http://hg.openjdk.java.net/jdk7/hotspot-comp/hotspot/rev/eca19a8425b5

I can not find any workspace that contains StringValue.java, nor can I 
find any CR the refers to removing it :( If all of this string caching 
stuff was removed on the JDK side the VM side was not cleaned up 
(perhaps due to hotspot express considerations.)

Even if this is all now obsolete, we unfortunately have a slight glitch 
to UseStringCache's removal as it seems to be a public exported 
interface - it is listed on the VMOptions page:

http://www.oracle.com/technetwork/java/javase/tech/vmoptions-jsp-140102.html

which means we need a CCC request to remove it, and we need a doc update 
for that web page (which doesn't indicate which JDK version it applies to.)

Or you can leave the stringcache code alone, and file a CR for someone 
to clean it up, sometime ;-)

David
-----

> I've re-run vm.quick on linux_x64 and built on windows_x64 and linux_x64.
> JPRT run is in progress.
>
> thanks,
> Calvin
>
> On 7/9/2013 7:19 PM, David Holmes wrote:
>> On 10/07/2013 10:14 AM, Calvin Cheung wrote:
>>> I've updated the webrev at the same location:
>>>      http://cr.openjdk.java.net/~ccheung/8004872/webrev/
>>
>> This is removing both the HashMap initialization and the StringValue
>> initialization - is that intended? You never mentioned StringValue and
>> it is not mentioned in the CR. Though StringValue does also seem to no
>> longer be present. In which case you can also remove:
>>
>> ./share/vm/classfile/vmSymbols.hpp: template(stringCacheEnabled_name,
>> "stringCacheEnabled")
>>
>> and it maybe that UseStringCache is also "dead" which means there is
>> further cleanup possible here:
>>
>> ./share/vm/opto/bytecodeInfo.cpp:  if (UseStringCache) {
>> ./share/vm/runtime/globals.hpp:  product(bool, UseStringCache, false,
>>
>>
>> It also looks like you can remove:
>>
>> ./share/vm/classfile/vmSymbols.hpp: template(frontCacheEnabled_name,
>> "frontCacheEnabled")
>>
>> David
>> -----
>>
>>> I did build on windows_x64 and linux_x64 and re-ran vm.quick on
>>> linux_x64.
>>>
>>> Calvin
>>>
>>> On 7/9/2013 2:16 PM, Calvin Cheung wrote:
>>>> Thanks for your confirmation.
>>>> I've just found the same and was about to reply.
>>>>
>>>> Calvin
>>>>
>>>> On 7/9/2013 2:14 PM, Christian Thalinger wrote:
>>>>> On Jul 9, 2013, at 1:58 PM, Calvin Cheung <calvin.cheung at oracle.com>
>>>>> wrote:
>>>>>
>>>>>> Thanks for your review.
>>>>>>
>>>>>> The HashMap class under jdk/src/share/classes/java/util is still
>>>>>> there.
>>>>>> So I'm not sure if we can remove that line in vmSymbols.hpp.
>>>>> You can; it's not used elsewhere:
>>>>>
>>>>> $ ack java_util_HashMap src/
>>>>> src/share/vm/classfile/vmSymbols.hpp
>>>>> 109:  template(java_util_HashMap,
>>>>> "java/util/HashMap")                        \
>>>>>
>>>>> src/share/vm/runtime/thread.cpp
>>>>> 3490:        Klass* tmp_k =
>>>>> SystemDictionary::find(vmSymbols::java_util_HashMap(), Handle(),
>>>>> Handle(), CHECK_0);
>>>>> 3493:        Klass* k_o =
>>>>> SystemDictionary::resolve_or_null(vmSymbols::java_util_HashMap(),
>>>>> Handle(), Handle(), CHECK_0);
>>>>>
>>>>> -- Chris
>>>>>
>>>>>> Calvin
>>>>>>
>>>>>> On 7/9/2013 1:10 PM, Ioi Lam wrote:
>>>>>>> Maybe this also can be removed?
>>>>>>>
>>>>>>> ./src/share/vm/classfile/vmSymbols.hpp: template(java_util_HashMap,
>>>>>>> "java/util/HashMap")                        \
>>>>>>>
>>>>>>> - Ioi
>>>>>>>
>>>>>>> On 07/09/2013 12:00 PM, Calvin Cheung wrote:
>>>>>>>> Please review a small fix:
>>>>>>>> http://cr.openjdk.java.net/~ccheung/8004872/webrev/
>>>>>>>>
>>>>>>>> Bug:
>>>>>>>> https://jbs.oracle.com/bugs/browse/JDK-8004872
>>>>>>>> (public bug not available)
>>>>>>>>
>>>>>>>> Summary:
>>>>>>>> The HashMap class under altclasses/java/util in the jdk repo.
>>>>>>>> was removed via another bug fix. Therefore, the initialization
>>>>>>>> code for
>>>>>>>> that class on the jvm side can also be removed.
>>>>>>>>
>>>>>>>> Testing:
>>>>>>>> JPRT
>>>>>>>> Aurora adhoc vm.quick on Mac OSX
>>>>>>>>     (other platforms in progress)
>>>>>>>>
>>>>>>>> thanks,
>>>>>>>> Calvin
>>>>
>>>
>