RFR: 8334866: Improve Speed of ElfDecoder source search [v6]

[Christian Hagedorn]

On Fri, 24 Oct 2025 17:38:35 GMT, Kerem Kat <krk at openjdk.org> wrote:

>> Right now, looking up source file and line number info is slow because we do a full linear scan of the `.debug_aranges` section for every single call. This can be a major bottleneck on large binaries, especially during frequent native stack walking, e.g. while writing an hs_err.
>> 
>> This change fixes that by caching the address ranges on the first lookup, and keeping it in memory for the lifetime of the `DwarfFile` object.
>> 
>> All subsequent lookups on that object now use a binary search instead of the slow linear scan. If caching fails for any reason, it just falls back to the old method.
>
> Kerem Kat has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix comments, delete copy ctor

Thanks for the updates and sharing some numbers! 

> Benchmark of PrintAssembly before/after PR, on a noisy intel laptop:

Can you show me where we are calling the DWARF parser when running with `-XX:+PrintAssembly`? Running without `-Xbatch` means that we sometimes compile more and sometimes less which will produce different sized outputs and thus quite some variance.

> Benchmarking an arbitrary test from TestDwarf.java, on a noisy laptop
[...]
    1.02 ± 0.03 times faster than <fastdebug-no-cache>
The test finishes ~35 ms faster with cache.

I hoped for some bigger improvement but still counts even when it's small.

I have some more comments and suggestions.

src/hotspot/share/utilities/elfFile.cpp line 761:

> 759:     }
> 760: 
> 761:     // Read all address descriptors for this set

Suggestion:

    // Read all address descriptors for this set into the cache.

src/hotspot/share/utilities/elfFile.cpp line 785:

> 783:           set_header._debug_info_offset
> 784:         );
> 785:         _cache._count++;

You do a lot of delegation to `_cache`. This suggests that it could be part of the cache class itself. How about having `add_entry()` with a `grow()` method there?

src/hotspot/share/utilities/elfFile.cpp line 801:

> 799:       _cache._capacity = _cache._count;
> 800:     }
> 801:   }

Why do we need this? Can you add a comment?

src/hotspot/share/utilities/elfFile.cpp line 803:

> 801:   }
> 802:   _cache._initialized = true;
> 803:   DWARF_LOG_INFO("Built aranges cache for '%s' with %zu entries", this->_dwarf_file->filepath(), _cache._count);

Suggestion:

  DWARF_LOG_INFO("Built .debug_aranges cache for '%s' with %zu entries", this->_dwarf_file->filepath(), _cache._count);

src/hotspot/share/utilities/elfFile.cpp line 812:

> 810: // This process is described in section 6.1.2 of the DWARF 4 spec.
> 811: bool DwarfFile::DebugAranges::find_compilation_unit_offset(const uint32_t offset_in_library, uint32_t* compilation_unit_offset) {
> 812:   switch(ensure_cached()) {

Suggestion:

  switch (ensure_cached()) {

src/hotspot/share/utilities/elfFile.hpp line 207:

> 205: #ifdef ASSERT
> 206:   const char* filepath() const { return _filepath; }
> 207: #endif

For one-liners, you could use `DEBUG_ONLY()`:

Suggestion:

  DEBUG_ONLY(const char* filepath() const { return _filepath; })

src/hotspot/share/utilities/elfFile.hpp line 442:

> 440:   };
> 441: 
> 442:   struct ArangesEntry {

You could add a comment here that this is an entry in the `ArangesCache`.

src/hotspot/share/utilities/elfFile.hpp line 452:

> 450:   };
> 451: 
> 452:   class DebugAranges;

Can you move this forward declaration to the top of the file where we already have some other forward declarations?

src/hotspot/share/utilities/elfFile.hpp line 456:

> 454:   // Cache for .debug_aranges to enable binary search for address lookup.
> 455:   // DebugAranges uses this cache to resolve the compilation_unit_offset, rather than doing a linear scan on the files
> 456:   // in each invocation of DebugAranges::find_compilation_unit_offset

Suggestion:

  // in each invocation of DebugAranges::find_compilation_unit_offset.

src/hotspot/share/utilities/elfFile.hpp line 466:

> 464:     bool _failed;
> 465: 
> 466:    public:

`public` is already implied here.

Suggestion:

src/hotspot/share/utilities/elfFile.hpp line 547:

> 545:       VALID,
> 546: 
> 547:       // Cache is unusable, fallback to linear scan.

Can you either add a comment here when a cache becomes unusable or add a reference where this is explained further?
Suggestion:

      // Cache is unusable, fall back to linear scan.

src/hotspot/share/utilities/elfFile.hpp line 568:

> 566:                               const AddressDescriptor& descriptor);
> 567:    public:
> 568:     DebugAranges(DwarfFile* dwarf_file) : _dwarf_file(dwarf_file), _cache(), _reader(dwarf_file->fd()),

Is `_cache()` really needed and not done implicitly?

src/hotspot/share/utilities/elfFile.hpp line 572:

> 570:     bool find_compilation_unit_offset(uint32_t offset_in_library, uint32_t* compilation_unit_offset);
> 571: 
> 572:     // Build cache of all address ranges for binary search in a single pass

For consistency with other method comments for the DWARF processing, I suggest to move the comment to the definition in the source file.

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

PR Review: https://git.openjdk.org/jdk/pull/27337#pullrequestreview-3382306631
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464932415
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464953428
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464901313
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464900663
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464824414
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464927421
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464804216
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464801391
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464802820
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464805799
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464811428
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464815856
PR Review Comment: https://git.openjdk.org/jdk/pull/27337#discussion_r2464818635