Skip to content

[llvm] Backport ORC dependence propagation performance fix - #23249

Open
pcanal wants to merge 1 commit into
root-project:masterfrom
pcanal:orc_loading_performance
Open

[llvm] Backport ORC dependence propagation performance fix#23249
pcanal wants to merge 1 commit into
root-project:masterfrom
pcanal:orc_loading_performance

Conversation

@pcanal

@pcanal pcanal commented Sep 3, 2026

Copy link
Copy Markdown
Member

Backport of upstream LLVM commit db5ffb04ab0b5c4e193d2c31e635e69f42deaaaf ("[ORC] WaitingOnGraph perf: faster dependence propagation.", PR llvm/llvm-project#183272), which is not present in the LLVM 22.

This fixes the intermittent timeout of rootest-root-io-cpp11Containers-unorderedMap.

Analysis

The roottest test root/io/cpp11Containers/unorderedMap (and related) showed a bistable runtime: the same test, in the same environment, would sometimes complete quickly and sometimes timeout (300s). With a debug build of LLVM we observed the test to usually run in ~800 seconds (40 seconds with a Release build) and sometimes take ~18 hours. Profiling attributed essentially all of the extra time to WaitingOnGraph::propagateSuperNodeDeps().

propagateSuperNodeDeps() computed, for every SuperNode, the transitive closure of its dependence set with a per-node DFS. Its total cost is

sum over X in closure(SN) of |Deps(X)| at the time X is expanded

which means the result depends critically on the order in which nodes are expanded:

  • If nodes happen to be visited in topological order, each node's dependants are already fully expanded when they are reached and the algorithm behaves like O(V + E).
  • If nodes happen to be visited in reverse topological order, each node re-expands a nearly-complete closure from scratch, degrading to roughly O(V^3).

The visitation order came from iterating a DenseMap keyed on SuperNode*, and DenseMapInfo<T*>::getHashValue() hashes the raw pointer value:

(unsigned((uintptr_t)P) >> 4) ^ (unsigned((uintptr_t)P) >> 9)

so the traversal order is a function of ASLR and heap layout. Two runs of the same binary on the same input could therefore land on opposite ends of that complexity range. The algorithm also mutated Deps in place while iterating (Deps = std::move(Reachable)) and de-duplicated work on pop rather than on push, which further inflated the worklist.

A second amplifier was sinkDeps(), which re-expanded the computed closures back into per-symbol dependence maps. This inflated the dependence sets and prevented the Coalescer from merging SuperNodes (coalescing requires exact equality of the dependence sets), so the graph stayed large and every subsequent emit paid the cost again.

Because both traversal orders compute the same closure, this never showed up as a correctness failure -- only as the 800s / 18h runtime split.

Solution

Adopt upstream's rewrite:

  • Invert the edges in SuperNodeDepsMap: SuperNodeDeps[SN] now holds the set of SuperNodes that depend on SN, so information flows forward along dependence edges instead of being pulled backwards by a DFS.
  • Replace the per-node reachable-set DFS with propagateDeps(), a monotone fixpoint that merges a node's dependence set into each of its dependants and only re-visits a dependant when its set actually grew. Convergence no longer depends on the initial worklist order, so the address-order sensitivity disappears.
  • Accumulate the closure directly into SuperNode::Deps, which removes the need for sinkDeps() entirely and keeps dependence sets small enough for the Coalescer to merge nodes.
  • Compute failure propagation transitively once, in the new propagateFailures(), instead of rediscovering it per node.
  • hoistDeps() now returns whether it modified the node, which lets emit() identify the pending SuperNodes whose dependence sets changed (and therefore must be removed from the coalescer) without a separate scan. processExternalDeps() returns a DenseSet<SuperNode *> and processReadyOrFailed() consumes it directly.

Validation

  • Differential test: 4000 randomised emit/fail scenarios driven through both the old and new implementations produce identical Ready and Failed sets (assertions enabled).

  • Performance on a synthetic dependence chain (time in simplify()):

    N       old        new       speedup
    500     0.0485s    0.0002s    221x
    1000    0.3762s    0.0005s    811x
    2000    1.9467s    0.0014s   1376x
    4000   21.4621s    0.0034s   6321x
    

    The old implementation scales ~8-11x per doubling of N; the new one ~2.4x.

  • libLLVMOrcJIT.a builds and links cleanly.

References

The fix first ships in LLVM 23 (verified present at tag llvmorg-23.1.0 and absent from release/22.x as of 22.1.8), so it cannot be picked up from an LLVM 22 point release. This patch can be dropped once ROOT's vendored LLVM moves to 23 or later.


Diagnosed, backported and validated with the assistance of Claude Opus 5.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 15h 36m 51s ⏱️
 3 865 tests  3 864 ✅ 0 💤 1 ❌
79 661 runs  79 660 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 7685bba.

♻️ This comment has been updated with latest results.

@hahnjo

hahnjo commented Sep 4, 2026

Copy link
Copy Markdown
Member

Thanks, the description should be heavily condensed, it's a lot of LLM bla bla

@dpiparo

dpiparo commented Sep 4, 2026

Copy link
Copy Markdown
Member

Thanks, the description should be heavily condensed, it's a lot of LLM bla bla

You mean the commit message, yes?
In that case, one could even use the original commit message...

…tion.

Backport of upstream LLVM commit db5ffb04ab0b5c4e193d2c31e635e69f42deaaaf
("[ORC] WaitingOnGraph perf: faster dependence propagation.", PR
llvm/llvm-project#183272), which is not present in the LLVM 22.

This fixes the intermittent timeout of rootest-root-io-cpp11Containers-unorderedMap.

The roottest test root/io/cpp11Containers/unorderedMap (and related)
showed a bistable runtime: the same test, in the same
environment, would sometimes complete quickly and sometimes timeout
(300s).  With a debug build of LLVM we observed the test to usually
run in ~800 seconds (40 seconds with a Release build) and sometimes take
~18 hours.

This commit replaces the core dependence propagation algorithm in
WaitingOnGraph to avoid worst-case behavior in the common case where
dependence graphs are sparse. This algorithm showed up as the underlying
cause of the bug in #179611.

For each call to MaterializationResponsibility::notifyEmitted,
WaitingOnGraph would build the transitive closure of all SuperNodes
whose "waiting on" relationships were affected by the newly emitted
symbols, then propagate any remaining unemitted dependencies through
this transitive closure graph. This approach is simple, but pushes the
algorithm towards n^2 complexity even for sparse dependence graphs.

The new propagation algorithm:
1. Inverts the edge direction in the SymbolDependenceMap data structure:
SymbolDepMap[SN] now contains the set of SuperNodes that depend on SN,
rather than the set that SN depends upon.

2. Pushes dependencies through the SymbolDepMap iteratively until it
reaches a fixed point.

This updated algorithm converges much more quickly than the original for
the testcase reported in the issue, and for other cases tested so far.

Validation
----------
  * Performance on a synthetic dependence chain (time in simplify()):

        N       old        new       speedup
        500     0.0485s    0.0002s    221x
        1000    0.3762s    0.0005s    811x
        2000    1.9467s    0.0014s   1376x
        4000   21.4621s    0.0034s   6321x

    The old implementation scales ~8-11x per doubling of N; the new one
    ~2.4x.

References
----------
  * Upstream commit: db5ffb04ab0b5c4e193d2c31e635e69f42deaaaf
  * Upstream PR:     llvm/llvm-project#183272
  * Upstream issue:  llvm/llvm-project#179611
  * Follow-up (perf regression infrastructure):
                     llvm/llvm-project#183251

The commit was released in LLVM 23.

---
Diagnosed, backported and validated with the assistance of Claude Opus 5.
@pcanal
pcanal force-pushed the orc_loading_performance branch from 7685bba to 2fe3d65 Compare September 4, 2026 15:15
@pcanal

pcanal commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

The commit log has been shortened (core is now the LLVM commit log).

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants