On Wed, 4 Mar 2026 16:56: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 primate 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 with a new target base due > to a merge or a rebase. The pull request now contains 16 commits: > > - Deleted obj_at and added obj_at_is_null > - Merge remote-tracking branch 'upstream_valhalla/lworld' into > lworld-remove-obj-at > - Merge remote-tracking branch 'upstream_valhalla/lworld' into > lworld-remove-obj-at > - Remove unused obj_at > - HotSpotJVMCI should use RefArrayOops internally, and expect exceptions > externally > - Fix ZERO > - Fix jvmti_GetThreadGroupChildren > - Fix ForeignGlobals > - Fix java_lang_invoke_MethodType > - Fix oop_verify_on > - ... and 6 more: > https://git.openjdk.org/valhalla/compare/a6b20a25...e417b2a5 Looks much safer and cleaner with your changes. However, this patch touches the code of the caches of the wrapper classes. Is this being done in coordination with with JDK-8379148 and JDK-8370724 ? ------------- Marked as reviewed by fparain (Committer). PR Review: https://git.openjdk.org/valhalla/pull/2157#pullrequestreview-3897192727
