RFR(M): 8223363: Bad node estimate assertion failure

[Patric Hedlin]

Thanks for reviewing Vladimir.

/Patric

On 2019-05-31 16:26, Vladimir Kozlov wrote:
> Looks good.
>
> thanks,
> Vladimir
>
> On 5/31/19 1:55 AM, Patric Hedlin wrote:
>> Updated testcase with "@requires !vm.graal.enabled".
>>
>> Refreshed Webrev: http://cr.openjdk.java.net/~phedlin/tr8223363/
>>
>> Best regards,
>> Patric
>>
>> On 29/05/2019 19:00, Vladimir Kozlov wrote:
>>>
>>> Hi Patric
>>>
>>> Add to test '@requires !vm.graal.enabled' to avoid running with Graal.
>>> And add link to test results to the bug report. I assume you need to 
>>> run 1-3 tiers and hs-precheckin-comp.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 5/29/19 12:48 AM, Patric Hedlin wrote:
>>>> Updated webrev: http://cr.openjdk.java.net/~phedlin/tr8223363/
>>>>
>>>> Now also including the testcase for/from 8223502.
>>>>
>>>> Best regards,
>>>> Patric
>>>>
>>>> On 28/05/2019 18:50, Aleksey Shipilev wrote:
>>>>> On 5/28/19 6:49 PM, Patric Hedlin wrote:
>>>>>> On 2019-05-28 15:51, Aleksey Shipilev wrote:
>>>>>>> On 5/28/19 3:07 PM, Patric Hedlin wrote:
>>>>>>>> Ooops, sorry. The test-case from 8223502 is here: Webrev:
>>>>>>>> http://cr.openjdk.java.net/~phedlin/tr8223502/
>>>>>>> Yes, so what prevents us from including that test in this 
>>>>>>> changeset? Surely it acts like the
>>>>>>> regression tests for the fix.
>>>>>> Absolutely nothing. The error was only in my webrev generation 
>>>>>> that didn't include both bookmarks.
>>>>>> They will be pushed "as one". (I can generate a new, single, 
>>>>>> complete, webrev in the morning if you
>>>>>> prefer.)
>>>>> Yes, please. I prefer full webrevs to understand what exactly is 
>>>>> being pushed.
>>>>>
>>>>> -Aleksey
>>>>>
>>>>
>>