RFR (S): 8201326: Renaming ThreadLocalAllocationBuffer end to fast_path_end
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Wed Apr 18 22:55:50 UTC 2018
Vladimir,
Thanks a lot for making this resolved!
It safes our time as we will stop all these renaming discussions now.
On 4/18/18 15:06, Vladimir Kozlov wrote:
> On 4/18/18 2:54 PM, JC Beyler wrote:
>> Seems like there is no consensus really except that changing it has a
>> few headaches involved. Should we then do the implementation of
>> "8171119: Low-Overhead HeapProfiling" without changing the _end field
>> and postpone this conversation to afterwards when Graal handles
>> multi-release in a more mature manner (read has been doing it for a
>> while).
>
> I would prefer this way.
+1
One more question is if we have to close this enhancement as NAI or to
keep it for post JEP renaming.
I'd prefer to close it.
>
> Thanks,
> Vladimir
>
>>
>> Or is there still a path forward? (Re)Naming is hard, this is proof
>> of that.
>>
>> That puts us back to what I had originally:
>> _end: the fast path end, can be the allocation end or the
>> to-be-sampled end
>> _allocation_end: the actual allocation end of the current TLAB
>> hard_end: _allocation_end + reserved space
Right.
It looks like the only way to move forward.
It will make it simpler.
Thanks,
Serguei
>>
>> Thanks for all of your input and help on this,
>> Jc
>>
>> On Wed, Apr 18, 2018 at 12:46 PM Vladimir Kozlov
>> <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>> wrote:
>>
>> On 4/18/18 12:15 PM, dean.long at oracle.com
>> <mailto:dean.long at oracle.com> wrote:
>> > I will defer to Vladimir, but I would have been happy with
>> something like:
>> >
>> > // preserve simple start/end abstraction for compiler
>> > HeapWord* end() const { return fast_path_end(); }
>> > static ByteSize end_offset() { return fast_path_end_offset(); }
>>
>> This is ugly.
>>
>> >
>> > even though end_offset() would then refer to a virtual "end"
>> field.
>> >
>> > Vladimir, are you also approving the Graal changes? :-)
>>
>> You really made my day :(
>>
>> S..t! We can't make this change in Graal as suggested because we
>> will break Graal's JDK 8 support.
>> Graal has direct access to VM's fields through JVMCI. You have to
>> guard change with JDK version
>> check.
>>
>> Labs start addressing multi-releases so it may be not big issue
>> anymore. See Doug's comment for
>> 8201318 RFR.
>>
>> Anyway you have to file new PR (pull request) for Graal changes on
>> https://github.com/graalvm/mx/pulls.
>> And I am not sponsoring this. I don't think such renaming worse
>> all our efforts.
>>
>> Good luck,
>> Vladimir
>>
>> >
>> > dl
>> >
>> > On 4/18/18 11:02 AM, Vladimir Kozlov wrote:
>> >> Ganging up on us ;)
>> >>
>> >> Yes, I missed original discussion about renaming on GC list.
>> >>
>> >> From my point of view next code looks better because it seems
>> compare related values:
>> >>
>> >> cmpptr(end, Address(thread, JavaThread::tlab_end_offset()));
>> >>
>> >> But I would not strongly object renaming to move this JEP
>> forward. Consider changes reviewed
>> and approved by me.
>> >>
>> >> Regards,
>> >> Vladimir
>> >>
>> >> On 4/18/18 6:29 AM, Daniel D. Daugherty wrote:
>> >>> Greetings,
>> >>>
>> >>> The idea of splitting this change off from "8171119:
>> Low-Overhead Heap Profiling"
>> >>> came up during the design review. It might have been me that
>> suggested the split
>> >>> off or it was someone else. Sorry I don't remember.
>> >>>
>> >>> In any case, the rename of "end" to "fast_path_end" is just
>> a clarity change to
>> >>> the existing code and I think that change can easily stand
>> on its own. That is
>> >>> particularly true if the accessors are renamed at the same
>> time. I think having
>> >>> the accessor names match the field names is a pretty well
>> known HotSpot rule
>> >>> so I'm not sure why we're talking about not renaming the
>> accessors...
>> >>>
>> >>> Personally, I prefer "_fast_path_end" over "_current_end".
>> >>>
>> >>> Dan
>> >>>
>> >>>
>> >>> On 4/18/18 4:04 AM, Stefan Johansson wrote:
>> >>>> Hi,
>> >>>>
>> >>>> On 2018-04-18 02:02, Vladimir Kozlov wrote:
>> >>>>> > I think I like better not to add it. If no one is using
>> it, there should be
>> >>>>> > no reason to add it? By not adding it, it also makes
>> any future wish to
>> >>>>> > have it a nice indicator of: hey why do you want to see
>> this? Same as
>> >>>>> > hard_end btw which is not visible. Am I missing something?
>> >>>>>
>> >>>>> I say "may" ;)
>> >>>>> You don't need new function if there is no use.
>> >>>>>
>> >>>>> >
>> >>>>> > So to summarize, the current consensus:
>> >>>>> > - _end can be renamed either to _current_end or
>> _fast_path_end; that is
>> >>>>> > still up to a vote and choice :)
>> >>>>>
>> >>>>> Please, use _current_end. I was thinking about _sample_end
>> but it does not reflect double
>> usage.
>> >>>>
>> >>>> I'm not sure if you have seen the discussion about naming
>> on hotspot-gc-dev, but I and
>> others think that
>> >>>> _current_end doesn't describe the usage at all. Naming it
>> _fast_path_end would clearly
>> state what it is, _sample_end
>> >>>> or something similar also crossed my mind but I think the
>> code reads a lot better in the
>> compiler with the name
>> >>>> fast_path_end.
>> >>>>
>> >>>>>
>> >>>>> > - the access method end() and tlab_end_offset()
>> remain the same to not
>> >>>>> > modify JIT/interpreter codes
>> >>>> I would find this very unfortunate, having accessors that
>> don't match the members can
>> easily lead to
>> >>>> misunderstanding, especially if we have three different
>> *_end members. Why do you think
>> this is the best way forward?
>> >>>>
>> >>>> My first thought was also that it would be nice to avoid
>> all the compiler changes, but
>> then we would have to stick
>> >>>> with _end being the sample/current/fast-path end and I'm
>> not sure that is a better
>> solution. I don't see the big
>> >>>> problem with changing those accessors to what they really
>> access and I think the compiler
>> code reads good even after
>> >>>> the change. For example:
>> >>>> cmpptr(end, Address(thread,
>> JavaThread::tlab_fast_path_end_offset()));
>> >>>> jcc(Assembler::above, slow_case);
>> >>>>
>> >>>>> >
>> >>>>> > If all agree to this, then the consequences are:
>> >>>>> > - JDK-8201326 becomes useless
>> >>>>> > - The work for JEP-331 becomes smaller in terms of
>> file changes
>> >>>>> > - I'll still be needing a decision on the renaming
>> of the TLAB _end field
>> >>>>> > (current suggestions are _current_end and _fast_path_end).
>> >>>>
>> >>>> We'll see where we end up. If JDK-8201326 ends up being a
>> separate change I think it
>> should be pushed at the same
>> >>>> time as the rest of the JEP changes. To me it only makes
>> sense to have it in the code base
>> if we also have the rest
>> >>>> of the changes.
>> >>>>
>> >>>> Thanks,
>> >>>> Stefan
>> >>>>
>> >>>>>
>> >>>>> Sounds good to me.
>> >>>>>
>> >>>>> Thanks,
>> >>>>> Vladimir
>> >>>>>
>> >>>>> On 4/17/18 4:51 PM, JC Beyler wrote:
>> >>>>>> Hi Vladimir and Dean,
>> >>>>>>
>> >>>>>> @Dean: seems that Vladimir is in agreement with you for
>> renaming just the
>> >>>>>> field and not the method names so ack to your answer that
>> came at the same
>> >>>>>> time :)
>> >>>>>>
>> >>>>>>
>> >>>>>>> Yes, from the beginning such changes should be discussed
>> on common
>> >>>>>>> hotspot-dev list since all
>> >>>>>>> Hotspot's parts are affected.
>> >>>>>>>
>> >>>>>>
>> >>>>>> Sorry, being new to the scene, most of the conversation
>> had been going on
>> >>>>>> in serviceability-dev.
>> >>>>>>
>> >>>>>>
>> >>>>>>>
>> >>>>>>> Graal specific question could be send to
>> hotspot-compiler-dev list with
>> >>>>>>> [Graal] in subject.
>> >>>>>>>
>> >>>>>>> I looked on JEP's changes
>> >>>>>>> http://cr.openjdk.java.net/~jcbeyler/8171119/webrev.02/
>> to understand how
>> >>>>>>> it works.
>> >>>>>>>
>> >>>>>>> Few questions about proposed JEP changes so I can
>> understand it.
>> >>>>>>>
>> >>>>>>> You introducing new TLAB end for sampling and adjust it
>> so that
>> >>>>>>> allocations in JITed code and
>> >>>>>>> Interpreter it will trigger slow path allocation where
>> you do sampling.
>> >>>>>>> Right?
>> >>>>>>>
>> >>>>>>
>> >>>>>> Yes that is correct; if sampling is enabled of course.
>> Btw, this is the current
>> >>>>>> webrev
>> <http://cr.openjdk.java.net/~jcbeyler/8171119/heap_event.15/>.
>> >>>>>>
>> >>>>>>
>> >>>>>>>
>> >>>>>>> Do all changes to _end, _actual_end and other TLAB
>> fields happen during
>> >>>>>>> slow allocation?
>> >>>>>>>
>> >>>>>>
>> >>>>>> Yes, to that effect, with Robbin's help, we finalized
>> deprecating the
>> >>>>>> FastTLabRefill via JDK-8194084
>> >>>>>> <https://bugs.openjdk.java.net/browse/JDK-8194084>. Seems
>> like I/we missed
>> >>>>>> that Graal does this as well. I filed GRAAL-64
>> >>>>>> <https://bugs.openjdk.java.net/browse/GRAAL-64> to not
>> forget that Graal
>> >>>>>> would need to get that fixed.
>> >>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>>>
>> >>>>>>> I am concern about concurrent accesses to these fields
>> from other threads
>> >>>>>>> if you have them (I don't
>> >>>>>>> see).
>> >>>>>>>
>> >>>>>>
>> >>>>>> Yes that is why we deprecated the FastTlabRefill. Other
>> threads should not
>> >>>>>> be changing the thread local data structure so I don't
>> see an issue there.
>> >>>>>> The major issue was that the fast paths could change the
>> tlab without going
>> >>>>>> via the slowpath.
>> >>>>>>
>> >>>>>> I had a fix to detect this issue but removed it once
>> JDK-8194084 was done;
>> >>>>>> Graal was missed in that work so that is why if sampling
>> were enabled with
>> >>>>>> Graal, there is a chance things would break currently.
>> That will get fixed
>> >>>>>> via GRAAL-64
>> <https://bugs.openjdk.java.net/browse/GRAAL-64> whether it is
>> >>>>>> rendering the code also obsolete and going to the
>> slowpath or whether it is
>> >>>>>> adding my fix again to detect a fastpath TLAB
>> reallocation happened.
>> >>>>>>
>> >>>>>>
>> >>>>>>>
>> >>>>>>> Renaming. I am fine with renaming if it helps to
>> understand code better. I
>> >>>>>>> agree with proposed
>> >>>>>>> changes to fields name:
>> >>>>>>>
>> >>>>>>> _current_end
>> >>>>>>> _allocation_end
>> >>>>>>>
>> >>>>>>> But, as Dean, I would suggest to keep names of access
>> functions. This way
>> >>>>>>> we can say that allocation
>> >>>>>>> code in Interpreter and JITed code stay the same.
>> >>>>>>>
>> >>>>>>
>> >>>>>> Sounds good to me, then in that case, this webrev will
>> disappear, which
>> >>>>>> also makes me happy, since it simplifies a lot of things
>> (and reduces code
>> >>>>>> change).
>> >>>>>>
>> >>>>>>
>> >>>>>>>
>> >>>>>>> You may need only new method to access _allocation_end
>> in places which
>> >>>>>>> look for real end of TLAB.
>> >>>>>>>
>> >>>>>>
>> >>>>>> I think I like better not to add it. If no one is using
>> it, there should be
>> >>>>>> no reason to add it? By not adding it, it also makes any
>> future wish to
>> >>>>>> have it a nice indicator of: hey why do you want to see
>> this? Same as
>> >>>>>> hard_end btw which is not visible. Am I missing something?
>> >>>>>>
>> >>>>>> So to summarize, the current consensus:
>> >>>>>> - _end can be renamed either to _current_end or
>> _fast_path_end; that is
>> >>>>>> still up to a vote and choice :)
>> >>>>>> - the access method end() and tlab_end_offset() remain
>> the same to not
>> >>>>>> modify JIT/interpreter codes
>> >>>>>>
>> >>>>>> If all agree to this, then the consequences are:
>> >>>>>> - JDK-8201326 becomes useless
>> >>>>>> - The work for JEP-331 becomes smaller in terms of
>> file changes
>> >>>>>> - I'll still be needing a decision on the renaming of
>> the TLAB _end field
>> >>>>>> (current suggestions are _current_end and _fast_path_end).
>> >>>>>>
>> >>>>>> Thanks for your help!
>> >>>>>> Jc
>> >>>>>>
>> >>>>>>
>> >>>>>>>
>> >>>>>>> Regards,
>> >>>>>>> Vladimir
>> >>>>>>>
>> >>>>>>> On 4/16/18 4:42 PM, JC Beyler wrote:
>> >>>>>>>> Hi Dean,
>> >>>>>>>>
>> >>>>>>>> I think perhaps this is also an attempt to figure out
>> the naming of all
>> >>>>>>>> this because naming (or renaming like here) is often
>> the start of big
>> >>>>>>>> conversations :).
>> >>>>>>>>
>> >>>>>>>> Originally, in the JEP-331 work, I had left the _end
>> field as is and, by
>> >>>>>>>> doing so, this whole renaming webrev goes away.
>> However, if we do that,
>> >>>>>>>> then the TLAB code contains:
>> >>>>>>>>
>> >>>>>>>> _end: the fast path end, can be the allocation end or
>> the to-be-sampled
>> >>>>>>> end
>> >>>>>>>> _allocation_end: the actual allocation end of the
>> current TLAB
>> >>>>>>>> hard_end: _allocation_end + reserved space
>> >>>>>>>>
>> >>>>>>>> With an early iteration of a webrev for JEP-331, a
>> conversation occurred
>> >>>>>>>> about whether or not that was clear or not and it was
>> determined that
>> >>>>>>> this
>> >>>>>>>> renaming was more clear:
>> >>>>>>>>
>> >>>>>>>> _current_end: the fast path end
>> >>>>>>>> _allocation_end: the actual allocation end of the
>> current TLAB
>> >>>>>>>> reserved_end: _allocation_end + reserved space
>> >>>>>>>>
>> >>>>>>>> Because I'm trying to reduce the footprint of files
>> changed, I pulled out
>> >>>>>>>> the renaming changes into this webrev. While talking
>> about it with the GC
>> >>>>>>>> team, the renaming suggested became:
>> >>>>>>>>
>> >>>>>>>> _fast_path_end: the fast path end
>> >>>>>>>> _allocation_end: the actual allocation end of the
>> current TLAB
>> >>>>>>>> hard_end: _allocation_end + reserved space
>> >>>>>>>>
>> >>>>>>>> Now, to be honest, any renaming or no renaming is fine
>> by me. I like not
>> >>>>>>>> renaming the fields to not change the code of every
>> backend and Graal; I
>> >>>>>>>> also like the fast_path_end rename due to it making the
>> backend code easy
>> >>>>>>>> to read as mentioned in the GC mailing lis thread.
>> >>>>>>>>
>> >>>>>>>> So there are two questions really:
>> >>>>>>>> - Should we rename the _end field and thus provoke
>> the changes in
>> >>>>>>> this
>> >>>>>>>> webrev?
>> >>>>>>>> - If we do want to change the field, should/could
>> it go in an initial
>> >>>>>>>> webrev or should it go in the JEP-331 webrev
>> whenever/ifever it goes in?
>> >>>>>>>> And if we do wait, could we at least converge on a
>> renaming now so that
>> >>>>>>>> this does not become a point of contention for that
>> webrev's review?
>> >>>>>>>>
>> >>>>>>>> If I read your answer correctly:
>> >>>>>>>> - You are saying that we should keep the _end
>> field altogether; or
>> >>>>>>> at
>> >>>>>>>> least only rename the field not the methods to access
>> it, thus reducing
>> >>>>>>> the
>> >>>>>>>> change scope.
>> >>>>>>>> - You are also saying to wait for the JEP-331
>> webrev's final
>> >>>>>>> iteration
>> >>>>>>>> and integrate it in one step
>> >>>>>>>>
>> >>>>>>>> Have I understood your answer correctly?
>> >>>>>>>>
>> >>>>>>>> Again, I am fine with renaming to whatever or not
>> renaming at all. I just
>> >>>>>>>> am trying to get forward progress here :)
>> >>>>>>>>
>> >>>>>>>> Thanks for your help!
>> >>>>>>>> Jc
>> >>>>>>>>
>> >>>>>>>> On Mon, Apr 16, 2018 at 3:29 PM <dean.long at oracle.com
>> <mailto:dean.long at oracle.com>>
>> wrote:
>> >>>>>>>>
>> >>>>>>>>> Hi JC. Others might disagree, but I would vote "no"
>> on doing this
>> >>>>>>>>> renaming now, before the JEP, and even in the context
>> of the JEP, I
>> >>>>>>>>> don't think renaming the field requires renaming all
>> the uses. The
>> >>>>>>>>> compiler code is only interested in the fast path, so
>> it's implicitly
>> >>>>>>>>> understood. Also, if there is a fast_path_end(), then
>> isn't it logical
>> >>>>>>>>> to have fast_path_start() paired with it? ("start" is
>> already
>> >>>>>>>>> overloaded, but nobody is suggesting adding
>> >>>>>>>>> allocation_start()/current_start()/hard_start() are
>> they?). My opinion
>> >>>>>>>>> is that it's fine the way it is.
>> >>>>>>>>>
>> >>>>>>>>> dl
>> >>>>>>>>>
>> >>>>>>>>> On 4/16/18 1:43 PM, JC Beyler wrote:
>> >>>>>>>>>> Hi all,
>> >>>>>>>>>>
>> >>>>>>>>>> I've left the mail thread from the hotspot-gc-dev
>> below for tracking
>> >>>>>>>>>> purposes but will start a new request here.
>> >>>>>>>>>>
>> >>>>>>>>>> - I'm trying to push a renaming of a TLAB field to
>> make my work for
>> >>>>>>>>> JEP-331
>> >>>>>>>>>> <http://openjdk.java.net/jeps/331> easier
>> >>>>>>>>>> - There is an understanding that if JEP-331
>> does not make it, this
>> >>>>>>>>> might
>> >>>>>>>>>> be useless and I'll revert if that is what the
>> community wants; however
>> >>>>>>>>> the
>> >>>>>>>>>> new name seems better anyway so perhaps not?
>> >>>>>>>>>>
>> >>>>>>>>>> - The webrev renames a field used across the various
>> back-ends and
>> >>>>>>> Graal
>> >>>>>>>>>> - The webrev is here:
>> >>>>>>>>>> http://cr.openjdk.java.net/~jcbeyler/8201326/webrev.04/
>> >>>>>>>>>>
>> >>>>>>>>>> Could I please get some feedback on this?
>> >>>>>>>>>>
>> >>>>>>>>>> Thanks all for your help,
>> >>>>>>>>>> Jc
>> >>>>>>>>>>
>> >>>>>>>>>>
>> >>>>>>>>>>
>> >>>>>>>>>> On Fri, Apr 13, 2018 at 2:37 AM Stefan Johansson <
>> >>>>>>>>>> stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>> wrote:
>> >>>>>>>>>>
>> >>>>>>>>>>> Hi JC,
>> >>>>>>>>>>>
>> >>>>>>>>>>> I don't have a name, but I've looked at bit more at
>> the failures and I
>> >>>>>>>>>>> think they are unrelated and one of the local
>> compiler engineers
>> >>>>>>> agree.
>> >>>>>>>>>>>
>> >>>>>>>>>>> I also ran some local testing and was not able to
>> get any error with
>> >>>>>>> you
>> >>>>>>>>>>> latest changes, but verified that it doens't work
>> without the graal
>> >>>>>>>>>>> parts. So they seem good. It might still be good to
>> switch this over
>> >>>>>>> to
>> >>>>>>>>>>> the general hotspot-dev list to let someone with
>> Graal knowledge to
>> >>>>>>> look
>> >>>>>>>>>>> at the change and make sure everything is correct.
>> >>>>>>>>>>>
>> >>>>>>>>>>> Thanks,
>> >>>>>>>>>>> Stefan
>> >>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>>> On 2018-04-12 17:26, JC Beyler wrote:
>> >>>>>>>>>>>> Hi Stefan,
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> Thanks for testing :). I've renamed the bug title
>> in the JBS and will
>> >>>>>>>>>>>> emit a new webrev shortly. Do you have the name of
>> a compiler
>> >>>>>>> engineer
>> >>>>>>>>>>>> in mind or should I perhaps now move this
>> conversation to the general
>> >>>>>>>>>>>> hotspot-dev mailing list and ask there?
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> The alternative is to add the compiler-mailing list
>> to this email
>> >>>>>>>>> thread
>> >>>>>>>>>>>> and ask there before sending to the general list.
>> What do you think
>> >>>>>>> is
>> >>>>>>>>>>> best?
>> >>>>>>>>>>>> Thanks for all your help,
>> >>>>>>>>>>>> Jc
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> On Thu, Apr 12, 2018 at 2:09 AM Stefan Johansson
>> >>>>>>>>>>>> <stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>
>> <mailto:stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>>>
>> >>>>>>>>>>> wrote:
>> >>>>>>>>>>>>
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> On 2018-04-11 17:48, JC Beyler wrote:
>> >>>>>>>>>>>> > Hi Stefan,
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> > Sorry about that, I must have missed adding the
>> files or
>> >>>>>>>>>>>> something. Here
>> >>>>>>>>>>>> > is a webrev that added the changes for the SA file:
>> >>>>>>>>>>>> >
>> http://cr.openjdk.java.net/~jcbeyler/8201326/webrev.03/
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> No problem, this looks good. I kicked of a
>> run in our test
>> >>>>>>> system
>> >>>>>>>>> to
>> >>>>>>>>>>>> get
>> >>>>>>>>>>>> some more coverage and have tried to include some
>> Graal
>> >>>>>>> testing.
>> >>>>>>>>> I'll
>> >>>>>>>>>>>> let you know the results once they are done.
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> Please also update the bug title both in JBS and
>> the changeset.
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> Cheers,
>> >>>>>>>>>>>> Stefan
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> > I changed the method name, which propagated a
>> change to:
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ObjectHeap.java
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> > I tried testing your test file. It exists in my
>> branch (if
>> >>>>>>> the
>> >>>>>>>>>>>> same) under:
>> >>>>>>>>>>>> > hotspot/jtreg/serviceability/sa/ClhsdbJhisto.java
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> > and interestingly (which generally means I did
>> something
>> >>>>>>>>> wrong),
>> >>>>>>>>>>> it
>> >>>>>>>>>>>> > passed before/after the change so I could not
>> verify that
>> >>>>>>> this
>> >>>>>>>>> is
>> >>>>>>>>>>>> a test
>> >>>>>>>>>>>> > showing that all is well in the world on my side.
>> Any ideas
>> >>>>>>> of
>> >>>>>>>>>>>> what I
>> >>>>>>>>>>>> > did wrong?
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> > I did again test it for
>> hotspot/jtreg/compiler/aot/ and
>> >>>>>>>>>>>> > hotspot/jtreg/serviceability/jvmti and it passes
>> those.
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> > Thanks for all your help,
>> >>>>>>>>>>>> > Jc
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> > On Wed, Apr 11, 2018 at 7:44 AM Stefan Johansson
>> >>>>>>>>>>>> > <stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com> <mailto:
>> >>>>>>>>> stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>>
>> >>>>>>>>>>>> <mailto:stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>
>> >>>>>>>>>>>> <mailto:stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>>>> wrote:
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> > Hi JC,
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> > On 2018-04-11 00:56, JC Beyler wrote:
>> >>>>>>>>>>>> > > Small update:
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > Here is the fixed webrev for the '{' that
>> were out of
>> >>>>>>>>>>>> alignment.
>> >>>>>>>>>>>> > This
>> >>>>>>>>>>>> > > passed release build JTREG
>> >>>>>>>>>>>> for hotspot/jtreg/compiler/jvmti (just
>> >>>>>>>>>>>> > to run
>> >>>>>>>>>>>> > > something as a smoke screen)
>> >>>>>>>>>>>> and hotspot/jtreg/compiler/aot/ (to
>> >>>>>>>>>>>> > test
>> >>>>>>>>>>>> > > Graal).
>> >>>>>>>>>>>> > >
>> >>>>>>> http://cr.openjdk.java.net/~jcbeyler/8201326/webrev.02/
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > I think this looks better, I agree that
>> leaving _end is
>> >>>>>>>>>>>> tempting to
>> >>>>>>>>>>>> > avoid a lot of change, but I think this will
>> be better
>> >>>>>>> in
>> >>>>>>>>> the
>> >>>>>>>>>>>> long run.
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> > I still miss the changes to make the SA work.
>> To see a
>> >>>>>>>>>>>> problem you
>> >>>>>>>>>>>> > can run:
>> >>>>>>>>>>>> > make CONF=fast run-test
>> >>>>>>>>>>>> >
>> >>>>>>>>>
>> TEST=open/test/hotspot/jtreg/serviceability/ClhsdbJhisto.java
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> > Cheers,
>> >>>>>>>>>>>> > Stefan
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>> > > Let me know what you think,
>> >>>>>>>>>>>> > > Jc
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > On Tue, Apr 10, 2018 at 3:21 PM JC Beyler
>> >>>>>>>>>>>> <jcbeyler at google.com <mailto:jcbeyler at google.com>
>> <mailto:jcbeyler at google.com <mailto:jcbeyler at google.com>>
>> >>>>>>>>>>>> > <mailto:jcbeyler at google.com
>> <mailto:jcbeyler at google.com>
>> <mailto:jcbeyler at google.com <mailto:jcbeyler at google.com>
>> >>>>>>>>>
>> >>>>>>>>>>>> > > <mailto:jcbeyler at google.com
>> <mailto:jcbeyler at google.com> <mailto:
>> >>>>>>> jcbeyler at google.com <mailto:jcbeyler at google.com>
>> >>>>>>>>>>
>> >>>>>>>>>>>> <mailto:jcbeyler at google.com
>> <mailto:jcbeyler at google.com>
>> <mailto:jcbeyler at google.com <mailto:jcbeyler at google.com>>>>>
>> >>>>>>>>> wrote:
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > Hi Karen and Stefan,
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > @Stefan: Naming is hard :)
>> >>>>>>>>>>>> > > @Karen: thanks for the Graal comment,
>> I fixed it
>> >>>>>>> in
>> >>>>>>>>>>>> the new
>> >>>>>>>>>>>> > webrev,
>> >>>>>>>>>>>> > > let me know what you think :)
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > I think the naming convention
>> suggested in this
>> >>>>>>>>> webrev
>> >>>>>>>>>>>> came from
>> >>>>>>>>>>>> > > conversations in for JEP-331 and I am
>> actually
>> >>>>>>>>>>> relatively
>> >>>>>>>>>>>> > > indifferent to the final result (as long
>> as we
>> >>>>>>> have
>> >>>>>>>>>>>> some form of
>> >>>>>>>>>>>> > > forward progress :)). To be honest,
>> I'd also be
>> >>>>>>>>> happy
>> >>>>>>>>>>>> to just
>> >>>>>>>>>>>> > leave
>> >>>>>>>>>>>> > > _end as is for all architectures and
>> Graal and
>> >>>>>>> have
>> >>>>>>>>> a
>> >>>>>>>>>>> new
>> >>>>>>>>>>>> > > _allocation_end. However, during initial
>> reviews
>> >>>>>>> of
>> >>>>>>>>>>>> JEP-331
>> >>>>>>>>>>>> > it was
>> >>>>>>>>>>>> > > deemed complicated to understand:
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > _end -> the _end or sampling end
>> >>>>>>>>>>>> > > _allocation_end -> end pointer for the
>> last
>> >>>>>>> possible
>> >>>>>>>>>>>> allocation
>> >>>>>>>>>>>> > > hard_end -> allocation end + reserved
>> space
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > That is how this naming came up and
>> why hard_end
>> >>>>>>>>> went
>> >>>>>>>>>>> to
>> >>>>>>>>>>>> > "reserved_end".
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > I'm really indifferent, so I offer as
>> a perusal:
>> >>>>>>>>>>>> > >
>> >>>>>>> http://cr.openjdk.java.net/~jcbeyler/8201326/webrev.01/
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > I noticed a few problems of alignement
>> of '{' so
>> >>>>>>>>> I'll
>> >>>>>>>>>>>> go fix
>> >>>>>>>>>>>> > that.
>> >>>>>>>>>>>> > > Basically this webrev does the following:
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > - Uses fast_path_end instead of end
>> >>>>>>>>>>>> > > - Reverts hard_end back to where it was
>> >>>>>>>>>>>> > > - Adds the changes to Graal; there is
>> a bunch of
>> >>>>>>>>>>>> changes in Graal
>> >>>>>>>>>>>> > > because Graal still contains a bit of
>> code doing
>> >>>>>>>>>>>> fasttlabrefills.
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > Let me know what you think!
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > Thanks,
>> >>>>>>>>>>>> > > Jc
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > On Tue, Apr 10, 2018 at 6:56 AM Karen
>> Kinnear
>> >>>>>>>>>>>> > > <karen.kinnear at oracle.com
>> <mailto:karen.kinnear at oracle.com>
>> >>>>>>>>>>>> <mailto:karen.kinnear at oracle.com
>> <mailto:karen.kinnear at oracle.com>> <mailto:
>> >>>>>>>>> karen.kinnear at oracle.com
>> <mailto:karen.kinnear at oracle.com>
>> >>>>>>>>>>>> <mailto:karen.kinnear at oracle.com
>> <mailto:karen.kinnear at oracle.com>>>
>> >>>>>>>>>>>> > <mailto:karen.kinnear at oracle.com
>> <mailto:karen.kinnear at oracle.com>
>> >>>>>>>>>>>> <mailto:karen.kinnear at oracle.com
>> <mailto:karen.kinnear at oracle.com>> <mailto:
>> >>>>>>>>> karen.kinnear at oracle.com
>> <mailto:karen.kinnear at oracle.com>
>> >>>>>>>>>>>> <mailto:karen.kinnear at oracle.com
>> <mailto:karen.kinnear at oracle.com>>>>>
>> >>>>>>>>>>>> > wrote:
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > Hi JC,
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > A comment about Graal. The impact
>> on Graal
>> >>>>>>> for
>> >>>>>>>>> this
>> >>>>>>>>>>>> > particular
>> >>>>>>>>>>>> > > change would be to break it - so
>> you’ll need
>> >>>>>>>>>>>> > > to complete the Graal changes for
>> this
>> >>>>>>> renaming.
>> >>>>>>>>>>>> That isn’t
>> >>>>>>>>>>>> > > optional or something that could be a
>> >>>>>>>>> follow-on. It
>> >>>>>>>>>>>> > > is not ok to break a feature, even an
>> >>>>>>>>> experimental
>> >>>>>>>>>>>> one.
>> >>>>>>>>>>>> > We will
>> >>>>>>>>>>>> > > discuss in the other thread potential
>> >>>>>>> phasing of
>> >>>>>>>>>>>> adding
>> >>>>>>>>>>>> > sampling.
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > I did not do a thorough search
>> -you can do
>> >>>>>>> that
>> >>>>>>>>> -
>> >>>>>>>>>>>> I did find
>> >>>>>>>>>>>> > > src/jdk.internal.vm.compiler/share/classes/
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java:
>> >>>>>>>>>>>> > > public final int threadTlabOffset =
>> >>>>>>>>>>>> > > getFieldOffset("Thread::_tlab",
>> >>>>>>> Integer.class,
>> >>>>>>>>>>>> > > "ThreadLocalAllocBuffer");
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java:
>> >>>>>>>>>>>> > > private final int
>> >>>>>>>>>>>> threadLocalAllocBufferStartOffset =
>> >>>>>>>>>>>> > >
>> >>>>>>> getFieldOffset("ThreadLocalAllocBuffer::_start",
>> >>>>>>>>>>>> > Integer.class,
>> >>>>>>>>>>>> > > "HeapWord*");
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java:
>> >>>>>>>>>>>> > > private final int
>> >>>>>>>>>>> threadLocalAllocBufferEndOffset =
>> >>>>>>>>>>>> > >
>> >>>>>>> getFieldOffset("ThreadLocalAllocBuffer::_end",
>> >>>>>>>>>>>> Integer.class,
>> >>>>>>>>>>>> > > "HeapWord*");
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java:
>> >>>>>>>>>>>> > > private final int
>> >>>>>>>>>>> threadLocalAllocBufferTopOffset =
>> >>>>>>>>>>>> > >
>> >>>>>>> getFieldOffset("ThreadLocalAllocBuffer::_top",
>> >>>>>>>>>>>> Integer.class,
>> >>>>>>>>>>>> > > "HeapWord*");
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java:
>> >>>>>>>>>>>> > > private final int
>> >>>>>>>>>>>> threadLocalAllocBufferPfTopOffset =
>> >>>>>>>>>>>> > >
>> >>>>>>>>> getFieldOffset("ThreadLocalAllocBuffer::_pf_top",
>> >>>>>>>>>>>> > Integer.class,
>> >>>>>>>>>>>> > > "HeapWord*");
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java:
>> >>>>>>>>>>>> > > private final int
>> >>>>>>>>>>>> > threadLocalAllocBufferSlowAllocationsOffset
>> >>>>>>>>>>>> > > =
>> >>>>>>>>>>>>
>> getFieldOffset("ThreadLocalAllocBuffer::_slow_allocations",
>> >>>>>>>>>>>> > > Integer.class, "unsigned");
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java:
>> >>>>>>>>>>>> > > private final int
>> >>>>>>>>>>>> > threadLocalAllocBufferFastRefillWasteOffset
>> >>>>>>>>>>>> > > =
>> >>>>>>>>>>>> >
>> >>>>>>>>>
>> getFieldOffset("ThreadLocalAllocBuffer::_fast_refill_waste",
>> >>>>>>>>>>>> > > Integer.class, "unsigned");
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java:
>> >>>>>>>>>>>> > > private final int
>> >>>>>>>>>>>> > threadLocalAllocBufferNumberOfRefillsOffset
>> >>>>>>>>>>>> > > =
>> >>>>>>>>>>>> >
>> >>>>>>>>>
>> getFieldOffset("ThreadLocalAllocBuffer::_number_of_refills",
>> >>>>>>>>>>>> > > Integer.class, "unsigned");
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java:
>> >>>>>>>>>>>> > > private final int
>> >>>>>>>>>>>> > > threadLocalAllocBufferRefillWasteLimitOffset
>> >>>>>>> =
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>>
>> getFieldOffset("ThreadLocalAllocBuffer::_refill_waste_limit",
>> >>>>>>>>>>>> > > Integer.class, "size_t");
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java:
>> >>>>>>>>>>>> > > private final int
>> >>>>>>>>>>>> > threadLocalAllocBufferDesiredSizeOffset =
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>>
>> getFieldOffset("ThreadLocalAllocBuffer::_desired_size",
>> >>>>>>>>>>>> > > Integer.class, "size_t");
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java:
>> >>>>>>>>>>>> > > public final int tlabAlignmentReserve =
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> getFieldValue("CompilerToVM::Data::ThreadLocalAllocBuffer_alignment_reserve",
>> >>>>>>>>>>>> > > Integer.class, "size_t”);
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > > hope this helps,
>> >>>>>>>>>>>> > > Karen
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> > >> On Apr 10, 2018, at 7:04 AM, Stefan
>> >>>>>>> Johansson
>> >>>>>>>>>>>> > >> <stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>
>> >>>>>>>>>>>> <mailto:stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>>
>> >>>>>>>>>>>> > <mailto:stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>
>> >>>>>>>>>>>> <mailto:stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>>>
>> >>>>>>>>>>>> > >> <mailto:stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>
>> >>>>>>>>>>>> <mailto:stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>>
>> >>>>>>>>>>>> > <mailto:stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>
>> >>>>>>>>>>>> <mailto:stefan.johansson at oracle.com
>> <mailto:stefan.johansson at oracle.com>>>>> wrote:
>> >>>>>>>>>>>> > >>
>> >>>>>>>>>>>> > >> Hi JC,
>> >>>>>>>>>>>> > >>
>> >>>>>>>>>>>> > >> I realize that the names have
>> been discussed
>> >>>>>>>>>>>> before but I'm
>> >>>>>>>>>>>> > >> leaning towards suggesting
>> something new. We
>> >>>>>>>>>>>> discussed this
>> >>>>>>>>>>>> > >> briefly here in the office and
>> others might
>> >>>>>>>>> have
>> >>>>>>>>>>>> different
>> >>>>>>>>>>>> > >> opinions. One point that came up
>> is that if
>> >>>>>>> we
>> >>>>>>>>> do
>> >>>>>>>>>>>> this
>> >>>>>>>>>>>> > change
>> >>>>>>>>>>>> > >> and change all usages of
>> >>>>>>>>>>>> JavaThread::tlab_end_offset() it
>> >>>>>>>>>>>> > >> would be good to make sure the
>> new name is
>> >>>>>>>>> good.
>> >>>>>>>>>>>> To us
>> >>>>>>>>>>>> > >> _current_end is not very descriptive, but
>> >>>>>>>>> naming
>> >>>>>>>>>>>> is hard and
>> >>>>>>>>>>>> > >> the best we could come up with is
>> >>>>>>>>> _fast_path_end
>> >>>>>>>>>>>> which would
>> >>>>>>>>>>>> > >> give the code:
>> >>>>>>>>>>>> > >> cmpptr(end, Address(thread,
>> >>>>>>>>>>>> > >> JavaThread::tlab_fast_path_end_offset()));
>> >>>>>>>>>>>> > >> jcc(Assembler::above, slow_case);
>> >>>>>>>>>>>> > >>
>> >>>>>>>>>>>> > >> I think this reads pretty good
>> and is fairly
>> >>>>>>>>>>>> clear. If we do
>> >>>>>>>>>>>> > >> this rename I think you can
>> re-use _end in
>> >>>>>>>>> JEP-331
>> >>>>>>>>>>>> > instead of
>> >>>>>>>>>>>> > >> calling it _allocation_end. But
>> that is a
>> >>>>>>> later
>> >>>>>>>>>>>> review :)
>> >>>>>>>>>>>> > >>
>> >>>>>>>>>>>> > >> Also, is there a good reason for
>> renaming
>> >>>>>>>>>>>> hard_end() to
>> >>>>>>>>>>>> > >> reserved_end()?
>> >>>>>>>>>>>> > >>
>> >>>>>>>>>>>> > >> One additional thing, you need to
>> update
>> >>>>>>> the SA
>> >>>>>>>>>>>> to reflect
>> >>>>>>>>>>>> > >> this change. I think the only
>> place you
>> >>>>>>> need to
>> >>>>>>>>>>>> fix is in:
>> >>>>>>>>>>>> > >>
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ThreadLocalAllocBuffer.java
>> >>>>>>>>>>>> > >>
>> >>>>>>>>>>>> > >> Thanks,
>> >>>>>>>>>>>> > >> Stefan
>> >>>>>>>>>>>> > >>
>> >>>>>>>>>>>> > >> On 2018-04-09 19:24, JC Beyler
>> 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>
>> >>>>>>> <https://teams.googleplex.com/u/where>
>> >>>>>>>>> <https://teams.googleplex.com/u/where>
>> >>>>>>>>>>> <https://teams.googleplex.com/u/where>
>> >>>>>>>>>>>> <https://teams.googleplex.com/u/where>
>> >>>>>>>>>>>> > <https://teams.googleplex.com/u/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?
>> >>>>>>>>>>>> > >
>> >>>>>>>>>>>> >
>> >>>>>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>>
>> >>>
>> >
>>
More information about the hotspot-dev
mailing list