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

Rahul Raghavan rahul.v.raghavan at oracle.com
Thu Jul 19 07:48:26 UTC 2018


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
>>
> 
> 


More information about the hotspot-runtime-dev mailing list