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

JC Beyler jcbeyler at google.com
Wed Jul 18 16:21:19 UTC 2018


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
>


-- 

Thanks,
Jc


More information about the hotspot-runtime-dev mailing list