On Thu, 30 May 2024 17:04:44 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
> I suggest leaving a comment to document which cases this cache is trying to > address. I think that's mainly cases where the same ValueLayout is created > many times in different places (instead of the same layout being reused, for > which we already have a cache field on the layout instance itself). Not quite. Note that since the recent changes, the handle we get from `Utils::makeSegmentViewVarHandle` is no longer cached *directly* on the value layout (because that handle is "raw" and has no size checks). What we cache is the adapted version of the handle which has the correct checks. When we create a var handle from some other root layout, we don't cache the resulting var handle anywhere - in part because doing so would be relatively expensive (you'd need a map from PathElement[] to VarHandle, and that's just for `varHandle`), and in part because reusing chance are rather low. That said, in both cases, to construct the "final" var handle, we have to create some raw var handle using `Utils::makeSegmentViewVarHandle`, so caching these raw handles seems to add up. But yes, all the above rationale should be captured in a comment. ------------- PR Comment: https://git.openjdk.org/jdk/pull/19485#issuecomment-2140981239
