On Thu, 5 Mar 2026 15:07:22 GMT, Axel Boldt-Christmas <[email protected]> wrote:
>> `obj_at(int)` on `flatArrayOopDesc` should never be called as it may fail >> with an out of memory error and crash. Similarly `objArrayOopDesc` should >> not provide a shared interface to this member function which is only valid >> to call on a `refArrayOopDesc`. >> >> This RFE cleans up the last potentially invalid uses by using and asserting >> stricter types at the call sites. >> >> Most cleanup are straight forward, however there are a few parts where some >> extra feedback is appreciated. >> >> First the primitive caches (ac32b67e811945ed474ac62d07275e8a464e72b9). >> [JDK-8369921](https://bugs.openjdk.org/browse/JDK-8369921)/#1685 conditioned >> the use of the caches behind the `!PreviewFeatures.isEnabled()` such that >> they are not used when running with Valhalla. However it did not do this for >> Character, even though both the issue and the PR mentions this Class as >> well, and could not find any discussion in the PR. >> >> Update: @jsikstro pointed me to >> [JDK-8372619](https://bugs.openjdk.org/browse/JDK-8372619), I am curious to >> where and how we access these caches (except for `valueOf`), as this change >> which removes the caches in Valhalla seems to run through our testing. >> >> Second the caches were still created even if they were not used. In this PR >> I chose to not create these caches at all if we are in preview mode. Also I >> am not sure if it is the case that we always trust the internal final values >> and do fold them, but I changed it to use `@stable` checks vs a sentinel, >> which I know are trusted as final. However maybe this is unnecessary, and >> some core library / compiler person has some input. >> >> Second is all the JVMCI changes (fe98ae0dc03c99c770dad1364fb1f010ea1ee7bf). >> This patch changes it so that the internal JVMCI Classes that that hotspot >> knows about are required to be RefArrays (this was already the assumption, >> but this hardens this by changing their types). And for external arrays they >> either throw an exception if it is an unexpected type (the case for >> `executeHotSpotNmethod`) or use `obj_at(int, TRAP)` and returns with an out >> of memory error. >> >> Testing: >> * Running tier 1-7 with `--enable-preview' > > Axel Boldt-Christmas has updated the pull request incrementally with one > additional commit since the last revision: > > Apply suggestion from @stefank > > Co-authored-by: Stefan Karlsson <[email protected]> test/hotspot/jtreg/runtime/cds/appcds/cacheObject/ArchivedIntegerCacheTest.java line 28: > 26: * @test > 27: * @summary Test primitive box caches integrity in various scenarios > (IntegerCache etc) > 28: * @requires !java.enablePreview I need to ask about that. When I run that (locally or not), before or after merging fresh lworld in this branch, I consistently get on this test Parse Exception: Syntax error in @requires expression: invalid name: java.enablePreview I see this syntax exist in other files, which I don't understand, but I can see it causes problems here. Is there something I'm missing? ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2157#discussion_r2894496544
