Skip to content

[geom] Reduce thread false sharing and improve multithreaded TGeo navigation - #22955

Open
sawenzel wants to merge 2 commits into
root-project:masterfrom
trwenz:twenzel/geom-false-sharing-fixes
Open

[geom] Reduce thread false sharing and improve multithreaded TGeo navigation#22955
sawenzel wants to merge 2 commits into
root-project:masterfrom
trwenz:twenzel/geom-false-sharing-fixes

Conversation

@sawenzel

@sawenzel sawenzel commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

This PR improves the multithreaded performance of TGeoNavigator by reducing the overhead of thread-local scratch data access and eliminating false sharing between threads.

The changes are the result of profile-guided optimization while profiling the parallel material budget scan in ALICE (see AliceO2Group/AliceO2#15641).

On a 28-core machine, the material budget LUT generation improves from a 12× to a 23× speedup (139 s → 72 s). Since the optimizations are in the geometry navigation infrastructure itself, they should benefit any application relying on multithreaded geometry algorithms.

This is the result of a summer internship of @trwenz on whose behalf the PR is opened.

Tristan Wenzel added 2 commits July 29, 2026 14:33
…igation

This commit improves multithreaded TGeo navigation. The changes come from profiling
a parallel geometry scan in ALICE: filling the material budget LUT on 28 cores now
scales from 12× to 23× speedup (139 s -> 72 s).

Two costs dominated. Every per-thread scratch lookup went through
TGeoManager::ThreadId(), a non-inlined cross-library __tls_get_addr call paid on
every boolean, section, and division query. In addition, the per-object
ThreadData_t blocks were allocated back-to-back, so slots belonging to different
threads often shared cache lines and invalidated each other on every write
(false sharing), especially across sockets.

Each object now gets a dense index, while the per-thread state lives in a single
thread_local vector indexed by it. GetThreadData() becomes a header-inlined TLS
read followed by an indexed load, and each thread owns its entire vector.

Main changes:
  * TGeoBoolNode, TGeoVolumeAssembly, TGeoPatternFinder, TGeoPgon, TGeoXtru
    now use indexed thread-local storage.
  * TGeoPgon/TGeoXtru: add noexcept move constructors for ThreadData_t (which
    owns heap buffers, and for TGeoXtru also a TGeoPolygon aliasing them), so
    entries remain valid when the vector grows.
  * TGeoPatternFinder: reuse the transformation matrix across generations. The
    matrix is owned by the geometry manager and is never released, so creating
    a new one would leak one matrix per (thread, finder).

Provisioning is no longer needed. ClearThreadData() now increments a generation
counter, and each thread lazily rebuilds its slot on first access. As a result,
CreateThreadData() and SetMaxThreads() are no longer required to support a given
thread count: any number of threads now works out of the box.

The generation counters are atomic because ClearThreadData() is const and may be
called concurrently.

Assisted-by: Claude Code (review, hardening and benchmarking)
Supervised-by: Sandro Wenzel <sandro.wenzel@cern.ch>
This commit provides a test exercising multithreaded TGeo navigation.

Eight threads navigating the same geometry must give exactly the single-threaded answer.
Covers TGeoXtru, TGeoPgon, TGeoVolumeAssembly, TGeoBoolNode (composite shape) and
TGeoPatternFinder (divided volume). The threads book their navigators lazily, so it also
exercises AddNavigator() against the navigator-map readers.

Assisted-by: Claude Code (review, hardening and benchmarking)
Supervised-by: Sandro Wenzel <sandro.wenzel@cern.ch>
@sawenzel

Copy link
Copy Markdown
Contributor Author
matbud_scaling

This graph demonstrates the improved scaling. It was measured on an older version of ROOT but it translates to the master version.

@github-actions

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 18h 56m 28s ⏱️
 3 883 tests  3 883 ✅ 0 💤 0 ❌
79 983 runs  79 983 ✅ 0 💤 0 ❌

Results for commit 9ebb0b7.

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.

2 participants