Joe Darcy | 17 May 23:45 2010
Picon

Re: HotSpot 17 merge

Andrew John Hughes wrote:
> On 17 May 2010 20:54, Joe Darcy <joe.darcy@...> wrote:
>   
>> Andrew John Hughes wrote:
>>     
>>> On 17 May 2010 18:08, Vladimir Kozlov
<vladimir.kozlov@...> wrote:
>>>
>>>       
>>>> Andrew,
>>>>
>>>> yes, you should revert/remove the 6909480 changeset in OpenJDK6 after
>>>> merge
>>>> with hs17
>>>>
>>>> Vladimir
>>>>
>>>> Andrew John Hughes wrote:
>>>>
>>>>         
>>>>> On 17 May 2010 17:29, Vladimir Kozlov <vladimir.kozlov@...>
>>>>> wrote:
>>>>>
>>>>>           
>>>>>> When we released jdk6u18 (hs16) there were 2 EA critical bugs
>>>>>> for which we did not have time to fix in 6u18:
>>>>>>
>>>>>> http://hg.openjdk.java.net/jdk7/hotspot-comp/hotspot/rev/f96a1a986f7b
>>>>>> http://hg.openjdk.java.net/jdk7/hotspot-comp/hotspot/rev/7fee0a6cc6d4
>>>>>>
>>>>>> In short, with escape analysis the generated code produces NULL(0)
>>>>>> instead of expected (stored) values.
>>>>>>
>>>>>> There was no enough time for full testing in 6u18 and changes were too
>>>>>> big.
>>>>>> I would advice to not use EA with HS16.
>>>>>>
>>>>>> In hs17 we enabled EA again and tested it until 6u21 release in which
>>>>>> the default flag's value is set to false but you can specify the flag
>>>>>> on command line to enable EA. There are few problems which needs to be
>>>>>> fixed before we enable EA by default.
>>>>>>
>>>>>> Andrew, you are right to not merge changes from hs16. And you can use
>>>>>> the flag on command line for HS17 instead of patch to enable EA.
>>>>>> It would be easier for benchmarking.
>>>>>>
>>>>>>
>>>>>>             
>>>>> Unfortunately it's not a matter of choosing not to merge the change,
>>>>> but having to revert it as it's already part of OpenJDK6 from the
>>>>> earlier hs16 merge (currently showing up as a difference between
>>>>> OpenJDK6's HotSpot and the hs17 branch).  See the results and
>>>>> discussion in the mail I just posted.
>>>>>
>>>>>
>>>>>           
>>>>>> Vladimir
>>>>>>
>>>>>> Andrew John Hughes wrote:
>>>>>>
>>>>>>             
>>>>>>> On 16 May 2010 18:22, Andrew Haley <aph@...> wrote:
>>>>>>>
>>>>>>>               
>>>>>>>> On 05/16/2010 05:14 PM, Andrew John Hughes wrote:
>>>>>>>>
>>>>>>>>                 
>>>>>>>>> The webrev is here:
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~andrew/jdk6-hs17-merge/
>>>>>>>>>
>>>>>>>>> I'd also like to revert:
>>>>>>>>>
>>>>>>>>> 6909480: Disable Escape Analysis in jdk 6u18
>>>>>>>>> http://hg.openjdk.java.net/hsx/hsx16/master/rev/4ebd3f8407eb
>>>>>>>>>
>>>>>>>>> which isn't present in hs17 (and so remains a difference between
>>>>>>>>> jdk6
>>>>>>>>> and hs17) as, unlike jdk6's version, hs17 is based on the main
>>>>>>>>> HotSpot
>>>>>>>>> tree rather than the hs16 master.  I didn't include this in the
>>>>>>>>> merge
>>>>>>>>> because it's clearer what's going on if it appears as a separate
>>>>>>>>> change.
>>>>>>>>>
>>>>>>>>>                   
>>>>>>>> Ahh, good catch.
>>>>>>>>
>>>>>>>> Do we have any idea why escape analysis is disabled?
>>>>>>>>
>>>>>>>> Andrew.
>>>>>>>>
>>>>>>>>
>>>>>>>>                 
>>>>>>> I assume it wasn't stable enough in hs16, but the bug isn't visible.
>>>>>>> Can someone from the HotSpot team shed more light on this?
>>>>>>>
>>>>>>>               
>>>>>           
>>> Yeah :-)
>>>
>>> Just waiting for the go-ahead from Joe now...
>>>
>>>       
>> Hello.
>>
>> I just finished looking over the lengthy patch.
>>
>> I approve the patch to update OpenJDK 6 to HS17 going back along with the
>> subsequence reverting of 6909480.
>>
>> Thanks,
>>
>> -Joe
>>
>>     
>
> I've pushed the merge.  I guess I'll need a bug ID to push the
> reversion.  Can you provide me with one?
>
>   

6953222 Revert change for 6909480 in OpenJDK 6

Cheers,

-Joe


Gmane