-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[hist] Implement initial RHistConcurrentFiller
#20555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Test Results 22 files 22 suites 3d 20h 21m 50s ⏱️ Results for commit 4eff656. ♻️ This comment has been updated with latest results. |
f523d62 to
b1c4939
Compare
| /// Create a new context for concurrent filling. | ||
| std::shared_ptr<RHistFillContext<BinContentType>> CreateFillContext() | ||
| { | ||
| std::lock_guard g(fMutex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One could maybe use the c++17 alternative:
| std::lock_guard g(fMutex); | |
| std::scoped_lock g(fMutex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understand, std::lock_guard is preferable for exactly one mutex.
b1c4939 to
2ff8e2a
Compare
jblomer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
In order to support efficient concurrent filling of RHist with global histogram statistics, we keep one local RHistStats object per RHistFillContext.
2ff8e2a to
4eff656
Compare
In order to support efficient concurrent filling of
RHistwith global histogram statistics, we keep one localRHistStatsobject perRHistFillContext.