On Wed, 6 May 2026 08:55:58 GMT, Tobias Hartmann <[email protected]> wrote:
>> To re-enable the autobox cache, this patch reverts (parts of) >> [JDK-8369921](https://bugs.openjdk.org/browse/JDK-8369921), >> [JDK-8378476](https://bugs.openjdk.org/browse/JDK-8378476), deopt parts of >> [JDK-8378531](https://bugs.openjdk.org/browse/JDK-8378531) and >> [JDK-8379148](https://bugs.openjdk.org/browse/JDK-8379148). >> >> I had to modify the `newCacheArray` methods to return a non-flattened array >> to avoid buffering. >> >> I wasn't sure about the comments above the `valueOf` methods: >> https://github.com/openjdk/valhalla/commit/b3ccb6abfdb6e2bff59a7d79bf01b669ac319257#diff-6136f7a80110ec00bf9a0b7da9943eb2fc33025ab400bc1f70153c1b2ac36993R146 >> >> Should they be reverted too? For now, I left them as is. >> >> Thanks, >> Tobias >> >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Tobias Hartmann has updated the pull request incrementally with one > additional commit since the last revision: > > Update src/hotspot/share/opto/type.cpp The code changes look good overall, thanks! Maybe @matias9927 knows more about the AOT-specifics here and can weigh in as well. I don't really have the expertise on specification to weigh in, but it seems like Victor has already pointed out some mistakes. src/hotspot/share/runtime/deoptimization.cpp line 1257: > 1255: BooleanBoxCache* BooleanBoxCache::_singleton = nullptr; > 1256: > 1257: oop Deoptimization::get_cached_box(AutoBoxObjectValue* bv, frame* fr, > RegisterMap* reg_map, bool& cache_init_error, TRAPS) { None of our box caches should be flat arrays. This should also apply to `BooleanBoxCache`, whose fields `TRUE` and `FALSE` should not be flattened. This is currently the case because we don't flatten `static` fields, but I think it'd be good to explicitly express these invariants as assertions somewhere (does not have to be here). Or even a comment saying to look at the JDKland code, so HotSpot readers see the absence of flattening. src/java.base/share/classes/java/lang/Character.java line 9442: > 9440: > 9441: // Load and use the archived cache if it exists > 9442: if (!PreviewFeatures.isEnabled()) { I don't think there is anything preventing us from using the CDS archive. We're currently archiving the `cache` even when running with `--enable-preview`, just not loading it. src/java.base/share/classes/java/lang/Integer.java line 952: > 950: cache = loadOrInitializeCache(precomputed); > 951: if (!PreviewFeatures.isEnabled()) { > 952: archivedCache = cache; // Legacy CDS archive support (to > be deprecated) I really wonder why this was guarded, if it's the same reason as the `CharacterCache` or because AOT specifically won't work with it. ------------- PR Review: https://git.openjdk.org/valhalla/pull/2390#pullrequestreview-4235556978 PR Review Comment: https://git.openjdk.org/valhalla/pull/2390#discussion_r3194967951 PR Review Comment: https://git.openjdk.org/valhalla/pull/2390#discussion_r3194979942 PR Review Comment: https://git.openjdk.org/valhalla/pull/2390#discussion_r3195010934
