RFR (S): 8201326: Renaming ThreadLocalAllocationBuffer end to current_end

JC Beyler jcbeyler at google.com
Mon Apr 9 20:57:47 UTC 2018


Hi Karen,

Fair enough and thanks for your help :) First time I ask for a GC change :)

So exactly: this is not a final review at all. I provide the webrev so
people can see what it looks like and that there is no confusion. Here are
some additional points:

- It really is not the final review since as you say, last time I had a
change that modified various architectures, I got help from different
people to do the testing on the different architectures that were affected.

- To answer the graal question, technically there are two graal changes
required:
   - A first one is due to this change, where the field name change will be
needed to be done on the graal side; and that is relevant to this change
8201326
   - Second is the fact Graal still does a FastTlabRefill and should not;
that is related to graal change 8171119; I had opened GRAAL-64
<https://bugs.openjdk.java.net/browse/GRAAL-64?filter=-1> for that change

So therefore, the question becomes a two-parter:
  - Would someone be willing to sponsor this change for me and help with
the tests?
  - Would that person and/or others able to perform the testing for the
various architectures want to help run a set of tests on the change?

Then, if people do agree to the design of the change and I can get a
sponsor, I can move this after initial testing (or after full testing) to
the main hotspot-dev email list.

Thanks again for all your help!
Jc


On Mon, Apr 9, 2018 at 1:16 PM Karen Kinnear <karen.kinnear at oracle.com>
wrote:

> JC,
>
> So I am the one who suggested that you ask the GC folks if they were ok
> with name change going in in advance,
> since the merging includes a number of files in a rapidly changing
> repository. Thank you for pointing out the risks.
>
> I am going to assume that this is a review for the approach, not the final
> source code review because:
> - I do not see the full set of tests run - which you would coordinate with
> your sponsor
>
> If the GC team is ok with the approach and you have all the tests passing
> - please send the actual code review request to
> hotspot-dev at openjdk.java.net. We use the team aliases for design
> consulting. We use the larger alias before anything goes in.
>
> See below ...
>
> On Apr 9, 2018, at 1:24 PM, JC Beyler <jcbeyler at google.com> wrote:
>
> Hi all,
>
> Small pre-amble to this request:
> In my work to try to get a heap sampler in OpenJDK (via JEP 331
> <https://bugs.openjdk.java.net/browse/JDK-8171119>), I'm trying to reduce
> the footprint of my change so that the integration can be easier. I was
> told that generally a JEP webrev should be feature complete and go in
> at-once. However, with the change touching quite a bit of various code
> pieces, I was trying to figure out what could be separated as not "part of
> the feature".
>
> I asked around and said that perhaps a solution would be to cut up the
> renaming of TLAB's end field that I do in that webrev. Because I'm renaming
> a field in TLAB used by various backends for that work, I have to update
> every architecture dependent code to reflect it.
>
> I entirely understand that perhaps this is not in the habits and very
> potentially might not be the way things are generally done. If so, I
> apologize and let me know if you would not want this to go in separately :)
>
> Final note: there is still a chance JEP-331 does not go in. If it does
> not, we can leave the new name in place or I'll happily revert it. I can
> even create an issue to track this if that makes it easier for all.
>
> End of the pre-amble.
>
>
> The 33-line change webrev in question is here:
> http://cr.openjdk.java.net/~jcbeyler/8201326/webrev.00/
>
> I fixed all the architectures and JVMCI and ran a few sanity tests to
> ensure I had not missed anything.
>
> Thanks for your help and I hope this is not too much trouble,
> Jc
>
> Ps: there is a graal change that needs to happen but I was not sure
> who/where <https://teams.googleplex.com/u/where> to ask about it. I was
> told it could happen in a separate webrev. Can anyone point me to the right
> direction? Should it just be hotspot-compiler-dev?
>
> Can I assume the graal change is not related to 8201326, but part of the
> 8171119 heap sampler collection email thread? That already
> includes the compiler team, so you should be set there.
>
> thanks,
> Karen
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20180409/a50d7976/attachment.html>


More information about the hotspot-gc-dev mailing list