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

[Stefan Johansson]

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 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?