Yes that is right, this is the latest: http://cr.openjdk.java.net/~jcbeyler/8207252/webrev.03/
I apologize for the multiple threads and confusion, Jc On Fri, Jul 20, 2018 at 11:22 AM serguei.spit...@oracle.com < serguei.spit...@oracle.com> wrote: > Thank you a lot, Vladimir! > Yes, the webrev.03 is the latest. > Jc, will correct us if it is not right. > > Thanks, > Serguei > > > On 7/20/18 10:52, Vladimir Kozlov wrote: > > I asked Igor V. to look. > > > > Seems like review is done in an other thread which does not have bug > > id in subject. Currently webrev.03 > > > > Vladimir > > > > On 7/19/18 4:32 PM, serguei.spit...@oracle.com wrote: > >> 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 <jcbey...@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 <jcbey...@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.barr...@oracle.com > > > >>>>>> wrote: > >>>>>> > >>>>>>>> On Jul 13, 2018, at 4:54 PM, JC Beyler <jcbey...@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.r...@oracle.com > > > >>>>>>> wrote: > >>>>>>>> > >>>>>>>>> On Jul 13, 2018, at 10:23 AM, JC Beyler <jcbey...@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
