RFR: 8362657: Make tables used in AOT assembly phase GC-safe [v2]

Aleksey Shipilev shade at openjdk.org
Tue Jul 22 06:19:50 UTC 2025


On Tue, 22 Jul 2025 02:32:44 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> This fixes crashes when using `-Xlog:aot+map=trace,aot+map+oops=trace:file=aot.oops.txt:none:filesize=0` during the assembly phase.
>> 
>> I basically changed a few raw `oop` pointers to `OopHandle`. I also simplified `ArchiveHeapWriter::is_marked_as_native_pointer()`.
>> 
>> Note that for `HeapShared::archived_object_cache()`, I calculate the hashcode using the raw oop. This way, we can avoid calling `oopDesc::identity_hash()` for every archived oops. As a result, when we use this table after exiting the CDS safepoint, we must rehash it. See changes in `ArchiveBuilder::CDSMapLogger::log()`.
>> 
>> - It might be alright to pre-calculate the identity hash of the archived oops in the assembly phase, but I think we should do that only if we find a reason to do so. We shouldn't do it just because it's convenient for implementing an internal table,
>
> Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Fixed 32-bit builds
>  - Merge branch 'premain' into 8362657-make-aot-assembly-tables-gc-safe
>  - Fixed white spaces
>  - 8362657: Make tables used in AOT assembly phase GC-safe

Not sure going for `OopHandle`-s is a right thing, since it introduces interesting leaks. See below. I think the alternative is to: a) make GC walk these collections as part of roots; b) maintain a separate collection that _only_ have `OopHandle`-ized oops as roots, leaving the maps holding the original `oop`-s (and allowing quieries with raw `oop`-s).

src/hotspot/share/cds/heapShared.hpp line 389:

> 387: 
> 388:   static CachedOopInfo* get_cached_oop_info(oop orig_obj) {
> 389:     OopHandle oh(&orig_obj);

`OopHandle`-s have no destructors, so this leaks?

-------------

PR Review: https://git.openjdk.org/leyden/pull/86#pullrequestreview-3041305033
PR Review Comment: https://git.openjdk.org/leyden/pull/86#discussion_r2221333667


More information about the leyden-dev mailing list