Skip to content

Use sorted vectors for DWARF address maps — reduce memory overhead#336

Open
lewing wants to merge 3 commits intodotnet/mainfrom
fix/dwarf-sorted-vectors-only
Open

Use sorted vectors for DWARF address maps — reduce memory overhead#336
lewing wants to merge 3 commits intodotnet/mainfrom
fix/dwarf-sorted-vectors-only

Conversation

@lewing
Copy link
Member

@lewing lewing commented Mar 16, 2026

Problem

wasm-opt uses excessive memory when processing wasm files with DWARF debug symbols, causing OOM on .NET CI Helix agents (dotnet/runtime#125244, dotnet/runtime#125233). The AddrExprMap and FuncAddrMap structures in wasm-debug.cpp use std::unordered_map with ~64 bytes per entry overhead.

Fix

Replace std::unordered_map with a SortedMap (sorted std::vector + binary search). These maps are built once during construction and only used for read-only lookups, making them ideal for this pattern.

Why it works

  • std::unordered_map: ~64 bytes/entry (hash buckets, linked list nodes, pointer chasing)
  • SortedMap (sorted vector): ~16 bytes/entry (contiguous memory, cache-friendly)
  • Binary search O(log n) is faster in practice than hash O(1) due to cache locality on sequential data
  • For a wasm binary with 1M expressions, this saves roughly 300-400MB of peak memory

Output is byte-for-byte identical, confirming functional correctness.

This is the first (and simplest) change from #335, split out for easier review.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces peak memory usage in DWARF debug processing by replacing address-to-expression/function std::unordered_map lookups with a compact sorted-vector map (SortedMap) that is built once and then queried read-only.

Changes:

  • Introduces SortedMap<K, V> (sorted std::vector + binary search) for address lookups.
  • Replaces AddrExprMap and FuncAddrMap internal std::unordered_map storage with SortedMap.
  • Adds up-front reservation and a post-build sort() step for the new maps.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Replace std::unordered_map with SortedMap (sorted vector + binary search)
for AddrExprMap and FuncAddrMap. These maps are built once during
construction and only used for read-only lookups, making them ideal
for this pattern.

- std::unordered_map: ~64 bytes/entry (hash buckets, linked list nodes)
- SortedMap (sorted vector): ~16 bytes/entry (contiguous, cache-friendly)

SortedMap tracks a finalized flag to assert lookups aren't performed
before sort(). Duplicate keys are de-duplicated after sorting (keeps
first) to handle cases like FuncAddrMap where start == declarations.

DelimiterLocations reservation now counts actual non-zero entries
rather than just the number of delimiter location arrays.

Output is byte-for-byte identical, confirming functional correctness.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lewing lewing force-pushed the fix/dwarf-sorted-vectors-only branch from 3b37476 to 11d339b Compare March 17, 2026 01:10
Copy link
Member Author

@lewing lewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed all three review comments:

  1. delimCount under-reservation — Now iterates each DelimiterLocations entry and counts actual non-zero offsets instead of just using delimiterLocations.size().

  2. Duplicate keys after sort()sort() now de-duplicates adjacent equal keys using std::unique (keeps first). This handles legitimate duplicates in FuncAddrMap where funcLocation.start == funcLocation.declarations for some functions.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces memory usage when processing wasm binaries with DWARF debug info by replacing per-address std::unordered_map lookups with a sorted std::vector-backed map and binary search, targeting OOM issues in large debug symbol workloads.

Changes:

  • Introduce a SortedMap<K, V> helper (vector + sort + binary search) for read-only address lookups.
  • Replace AddrExprMap and FuncAddrMap internal std::unordered_map structures with SortedMap, including pre-reservation and finalization (sort()).
  • Update lookup code paths to use the new find() API and pointer-based returns.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

- Remove SortedMap::count() — no callers remain after removing
  pre-sort assertions, and it could be misused during build phase.
- Document that duplicate keys (e.g. FuncAddrMap start==declarations)
  always map to the same value, so de-dup order is irrelevant.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces peak memory usage when processing DWARF debug info by replacing address→node/function hash maps with a sorted-vector-based lookup structure in wasm-debug.cpp, leveraging the fact that these maps are constructed once and then queried read-only during DWARF rewriting.

Changes:

  • Introduce a SortedMap (sorted std::vector + binary search) for address-keyed lookups.
  • Replace std::unordered_map usages in AddrExprMap and FuncAddrMap with SortedMap.
  • Pre-reserve vector capacity and finalize maps via sorting/deduplication after construction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Add debug-time validation in SortedMap::sort() that duplicate keys
map to the same value (assertUniqueValues=true by default).

FuncAddrMap passes assertUniqueValues=false because contiguous
functions legitimately share boundary addresses (func1.end ==
func2.start), matching the old unordered_map overwrite behavior.

AddrExprMap uses the default (true) to catch debug info issues early.

Also adds operator== to DelimiterInfo for the assertion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Member Author

@lewing lewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both remaining comments addressed in 68b89a6:

  • sort() duplicate validation (line 392): sort() now takes assertUniqueValues param (default true). In debug builds, asserts that duplicate keys map to the same value before de-duplicating. AddrExprMap uses the default strict check; FuncAddrMap passes false since contiguous functions legitimately share boundary addresses (func1.end == func2.start).

  • AddrExprMap uniqueness (line 478): The strict assertion (assertUniqueValues=true) now validates during sort() that startMap and endMap entries have unique keys (or identical values for any duplicates), preserving the same invariant as the old pre-sort assertions.

@radekdoulik
Copy link
Member

Binary search O(log n) is faster in practice than hash O(1) due to cache locality on sequential data

I am curious, do you have some measurements for this? What is a typical size of the data, how many entries?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants