[geom] Reduce thread false sharing and improve multithreaded TGeo navigation - #22955
Merged
agheata merged 7 commits intoSep 4, 2026
Merged
Conversation
Contributor
Author
This graph demonstrates the improved scaling. It was measured on an older version of ROOT but it translates to the master version. |
Test Results 23 files 23 suites 3d 15h 48m 17s ⏱️ For more details on these failures, see this check. Results for commit 8b6be5a. ♻️ This comment has been updated with latest results. |
silverweed
reviewed
Jul 30, 2026
agheata
requested changes
Sep 1, 2026
agheata
left a comment
agheata
left a comment
Member
There was a problem hiding this comment.
Thanks for this excellent optimization work, with substantial improvements that I could verify. It would be good to have some changes only around lifetime and ownership aspects introduced by the new TLS design:
TGeoPgonandTGeoXtruTLS entries retain their heap allocations until the worker thread exits, rather than releasing them when the corresponding geometry object is destroyed. The fast TLS lookup can be preserved by making these entries non-owning caches while the geometry object owns and releases the allocations (see suggestions inline)- Lazy
TGeoPatternFindermatrix creation relies on the current activegGeoManager/gGeoIdentity. With multiple managers, a finder belonging to A can therefore cache a matrix owned by B. Creation and registration should explicitly usefVolume->GetGeoManager() - Monotonic indices create a process-lifetime TLS high-water mark. This may be acceptable if repeated geometry churn is out of scope, but the intended lifecycle should be documented.
- The statement about no longer requiring
SetMaxThreads()should be limited to scratch-data provisioning, since navigator registration still uses it to enable thread safety.
agheata
reviewed
Sep 1, 2026
Contributor
Author
|
Fixed the merge conflict with master. As discussed privately with @agheata, this PR can be amended by maintainers in order to arrive at best possible solution for TGeo and to address the suggestions directly. |
added 2 commits
September 1, 2026 11:45
…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
force-pushed
the
twenzel/geom-false-sharing-fixes
branch
from
September 1, 2026 09:45
bc2f458 to
3e31b23
Compare
This was referenced Sep 1, 2026
Lazy pattern initialization can run while a different geometry manager is current. Register new matrices, including identity matrices, with the manager owning the divided volume so deleting an unrelated manager cannot invalidate the TLS cache. Preserve the existing CreateMatrix interface and cover the two-manager lifetime explicitly.
Keep the hot TLS slots for Pgon and Xtru as non-owning caches while moving their large buffers and polygons under shape ownership. ClearThreadData and shape destruction can now reclaim these allocations once navigation using the shape has stopped. Cover cleanup and lazy rebuilding from stale TLS slots with multiple worker threads.
Document that monotonic TLS slot vectors retain their high-water size until the owning thread exits, while the shape-owned large allocations are reclaimed separately. Limit lazy-allocation claims to component scratch state and clarify that SetMaxThreads is still required for thread-safe manager navigation. Correct the cached pattern-matrix lifetime description as well.
Summarize the improved multithreaded TGeo navigation and the reported ALICE benchmark. Credit Sandro Wenzel and Tristan Wenzel, with Tristan's affiliation corrected to ETHZ.
agheata
self-requested a review
September 2, 2026 14:00
agheata
approved these changes
Sep 2, 2026
agheata
left a comment
agheata
left a comment
Member
There was a problem hiding this comment.
I think now all issues were addressed, and this is ready to go
TGeoPgon::GetThreadData() and TGeoXtru::GetThreadData() are inline, so consumer translation units directly reference fgInstanceCount. On Windows, automatically exporting DLL symbols does not provide the dllimport semantics required for static data, resulting in unresolved external symbols when linking clients. The counter was only used to pre-size the TLS vector. Resize it to fIndex + 1 instead, which is sufficient to access the object’s non-reused slot and avoids the DLL data reference.
Member
|
CI failures are unrelated: one graphics test and a spurious build failure. Merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.