RFR (S) 8207252: C1 still does eden allocations when TLAB is enabled

[serguei.spitsyn at oracle.com]

Thanks, Rahul!
In fact, there no good experts for this area in the serviceability team.
It would be much better if anyone from the Compiler team could do it.

Vladimir K.,

Is there anyone from the Compiler team available to review this?
Otherwise, I could try to review it but am not sure about my review quality.

Thanks,
Serguei


On 7/19/18 00:48, Rahul Raghavan wrote:
> RFR (S) 8207252: C1 still does eden allocations when TLAB is enabled
>
> (just adding + hotspot-compiler-dev also)
>
>
> On Wednesday 18 July 2018 09:51 PM, JC Beyler wrote:
> Subject Was:
> Re: RFR (S): C1 still does eden allocations when TLAB is enabled
>
> + serviceability-dev
>
> Hi all,
>
> Could anyone else give me a review of this webrev and check/test the
> various architecture changes?
>
> http://cr.openjdk.java.net/~jcbeyler/8207252/webrev.02/
>
>
> Thanks for all your help!
> Jc
>
>
>> On Mon, Jul 16, 2018 at 2:58 PM JC Beyler <jcbeyler at google.com> wrote:
>>
>>> Hi all,
>>>
>>> Here is a webrev that does all the architectures in the same way:
>>> http://cr.openjdk.java.net/~jcbeyler/8207252/webrev.02/
>>>
>>> Could anyone review the other architectures and test?
>>>    - arm, sparc & aarch64 are also modified now to follow the same 
>>> "if no
>>> tlab, then consider eden space allocation" logic.
>>>
>>> Thanks for your help!
>>> Jc
>>>
>>> On Fri, Jul 13, 2018 at 9:16 PM JC Beyler <jcbeyler at google.com> wrote:
>>>
>>>> Hi Kim,
>>>>
>>>> I opened this bug
>>>> https://bugs.openjdk.java.net/browse/JDK-8190862
>>>>
>>>> and now I've done an update:
>>>> http://cr.openjdk.java.net/~jcbeyler/8207252/webrev.01/
>>>>
>>>> I basically have done your nits but also removed the try_eden (it was
>>>> used to bind a label but was not used). I updated the comments to 
>>>> use the
>>>> one you preferred.
>>>>
>>>> I still have to do the other architectures though but at least we 
>>>> seem to
>>>> have a consensus on this architecture, correct?
>>>>
>>>> Thanks for the review,
>>>> Jc
>>>>
>>>> On Fri, Jul 13, 2018 at 5:17 PM Kim Barrett <kim.barrett at oracle.com>
>>>> wrote:
>>>>
>>>>>> On Jul 13, 2018, at 4:54 PM, JC Beyler <jcbeyler at google.com> wrote:
>>>>>>
>>>>>> Yes, you are right, I did those changes due to:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8194084
>>>>>>
>>>>>> If Robbin agrees to this change, and if no one sees an issue, 
>>>>>> I'll go
>>>>> ahead
>>>>>> and propagate the change across architectures.
>>>>>>
>>>>>> Thanks for the review, I'll wait for Robbin (or anyone else's 
>>>>>> comment
>>>>> and
>>>>>> review) :)
>>>>>> Jc
>>>>>>
>>>>>> On Fri, Jul 13, 2018 at 1:08 PM John Rose <john.r.rose at oracle.com>
>>>>> wrote:
>>>>>>
>>>>>>> On Jul 13, 2018, at 10:23 AM, JC Beyler <jcbeyler at google.com> 
>>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>> I'm not sure if we had left this case intentionally or not but, 
>>>>>>> if we
>>>>> want
>>>>>>> it all to be consistent, we should perhaps fix it.
>>>>>>>
>>>>>>>
>>>>>>> Well, you put in that logic last February, so unless somebody 
>>>>>>> speaks
>>>>> up
>>>>>>> quickly, I support your adjusting it to be the way you want it.
>>>>>>>
>>>>>>> Doing "hg grep -u supports_inline_contig_alloc -I 
>>>>>>> src/hotspot/share"
>>>>>>> suggests that the GC group is most active in touching this feature.
>>>>>>> If Robbin is OK with it, there's your reviewer.
>>>>>>>
>>>>>>> FWIW, you can use me as a reviewer, but I'd get one other person
>>>>>>> working on the GC to OK it.
>>>>>>>
>>>>>>> — John
>>>>>>>
>>>>>>
>>>>>>
>>>>>> -- 
>>>>>>
>>>>>> Thanks,
>>>>>> Jc
>>>>>
>>>>> Robbin is on vacation; you might not hear from him for a while.
>>>>>
>>>>> I'm assuming you'll open a new bug for this?
>>>>>
>>>>> Except for a few minor nits (below), this looks okay to me.
>>>>>
>>>>> The comment at line 1052 needs updating.
>>>>>
>>>>> pre-existing: The retry_tlab label declared on line 1054 is unused.
>>>>>
>>>>> pre-existing: The try_eden label declared on line 1054 is bound at
>>>>> line 1058, but unreferenced.
>>>>>
>>>>> I like the wording of the comment at 1139 better than the wording at
>>>>> 1016.
>>>>>
>>>>>
>>>>
>>>> -- 
>>>>
>>>> Thanks,
>>>> Jc
>>>>
>>>
>>>
>>> -- 
>>>
>>> Thanks,
>>> Jc
>>>
>>
>>