On Tue, 3 Mar 2026 14:16:02 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 14 commits: > > - 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 > - Fix BacktraceBuilder and java_lang_Throwable > - oopFactory::new_objectArray always creates refArrayOop > - ... and 4 more: > https://git.openjdk.org/valhalla/compare/fb166a6d...2dd346f3 I went through but that's too much runtime-related for me to say much. Indeed, most of the changes seems very direct. And thanks for doing that, it will be less confusing. I've used `flatArrayOopDesc::obj_at(int)` before the `fatal` was added. I just wonder whether we should add a warning on `flatArrayOopDesc::obj_at(int index, TRAPS)` quickly stating why the `obj_at(int)` version doesn't exist so nobody is tempted to reintroduce it. It's tempting, the `..., TRAPS)` version often have a version without. Until Frederic explained me what happens there, I think I could have (re-)added it. ------------- PR Comment: https://git.openjdk.org/valhalla/pull/2157#issuecomment-3996809020
