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





Reply via email to