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

[JC Beyler]

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