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

[JC Beyler]

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