RFR: 8356173: Remove ThreadCritical [v5]
Coleen Phillimore
coleenp at openjdk.org
Tue May 13 11:58:06 UTC 2025
On Mon, 12 May 2025 23:09:52 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>> I still fail to see the point of this. dynamic allocation isn't something that's a problem for this object. It may be for the NMT types that use this wrapper but not for this object. You cannot write a microbenchmark to show the effect of this pointer indirection in this code path. If you could, there's something seriously wrong with our algorithms which should be fixed instead. The initialization check is an obvious one liner, not executable in production.
>>
>> The DeferredWhat? wrapper is extra typing, that requires you to go to some other place to figure out that among all the casting, it's doing a placement new. For no reason other than to give you more code to navigate to.
>
> Clearly we disagree about the utility of Deferred. I won't insist on it being
> used here, even though I think it's a fine use-case. I think this comment from
> the PR best describes the motivation:
> https://github.com/openjdk/jdk/pull/24689#issuecomment-2823556707
> Unfortunately, that comment is kind of buried down in the middle of the PR.
>
> But some of the criticisms seem incorrect. For example, using Deferred is less
> typing, even without including the assert against multiple initialization that
> is missing from the current code. (That's one of the problems with
> boiler-plate - it's easy to forget or mess up some of it.) And I don't know
> what you mean by "all the casting", as there is precisely one (const) cast in
> Deferred. Maybe you are remembering earlier versions from the PR that added
> it? (Some versions did have lots of casts.)
As an aside, I like the StableValue name better than Deferred, as the latter means nothing to me. So changing to Deferred would just defer my attention to another file to find out what Deferred means, when this is literally 5 lines of code in front of me. So I would not like to change it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25072#discussion_r2086638360
More information about the hotspot-dev
mailing list