[geom] Complete TLS ownership and cleanup - #2
Merged
sawenzel merged 4 commits intoSep 2, 2026
Merged
Conversation
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.
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
2 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.
Motivation
This follow-up addresses ownership and cleanup issues identified during the review of root-project#22955. It keeps the revised thread-local-data access model while making its behavior well-defined for multiple geometry managers and geometry cleanup.
Changes
TGeoCombiTransrotations, with the manager owning the divided volume. This makes concurrent navigation of multiple geometry managers independent ofgGeoManager.TGeoPgonandTGeoXtruown their dynamically allocated per-thread buffers. TLS entries retain only non-owning views, allowingClearThreadData()and object destruction to release the storage.SetMaxThreads()contracts.Validation
The focused
thread_navigationsuite passes all three tests, covering multithreaded navigation, matrix ownership across two live geometry managers, and release ofTGeoPgon/TGeoXtruscratch storage.Performance was compared with the rebased root-project#22955 head (
3e31b2338b9) on an AMD Ryzen 9 7950X3D using GCC 13.3,RelWithDebInfo, pinned physical cores, and the median of five alternating runs. Mixed-geometry navigation remained within measurement noise: 4.626 s before versus 4.581 s after at one thread (-1.0%), and 4.679 s versus 4.721 s at eight threads (+0.9%). An isolated scratch-data-access benchmark improved from 3.346 s to 3.035 s for two billion accesses (-9.3% runtime, +10.2% throughput). A synthetic clear/recreate benchmark increased from 4.467 s to 4.743 s for 20 million cycles (+6.2%), as expected because cleanup now performs the previously missing deallocations.Assisted by Codex (GPT-5).