On Mon, 3 Apr 2023 13:57:52 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:

>>> This looks interesting. I think it is time to rename eetop to 
>>> javaThreadAddr ...
>> 
>> Feel free to file a RFE but not as part of this PR. :)
>
>> > This looks interesting. I think it is time to rename eetop to 
>> > javaThreadAddr ...
>> 
>> Feel free to file a RFE but not as part of this PR. :)
> 
> Well, when this thing is considered in isolation, it changes the clear 
> `isAlive0()` call to rather obscure `eetop != 0` condition, with no 
> documentation what that field actually is. At very least we should add the 
> comment that the field carries the native thread pointer, and `!= 0` is 
> actually checking for `null`.
> 
> But, someone (who should remain anonymous under the alias, say... rolls 
> dice... Andrei?) pointed out that async-profiler looks up `eetop`, so the 
> rename would break the profiler until the renamed field is handled there. So 
> the rename at this point is a bit dicey: 
> https://github.com/async-profiler/async-profiler/blob/39b0bdb5e446b1b88acdf37fd18a4560f5176a2e/src/vmStructs.cpp#L409

Thanks for the review @shipilev !

-------------

PR Comment: https://git.openjdk.org/jdk/pull/13287#issuecomment-1497115716

Reply via email to