ROOT I/O changes to reduce wait time on global lock #747
Conversation
|
Starting build on |
|
The attached image shows a summary of differences on KNL with 128 threads. Notice how the wait time is significantly reduced from 1485.7s to 1105s. There are also 2936 less waits than before. CPU time of all threads is reduced by ~50s. These are not hugely visible in real runtime, because we are limited by I/O to the disk, essentially, but it would be clearly visible if I/O ceases to be the main bottleneck. The two figures on the bottom show that the time during which the threads run is shorter after the changes introduced in this PR. For reference, the running time without using VTune is about 40s.
|
|
Starting build on |
|
Starting build on |
|
Starting build on |
|
Starting build on |
|
Starting build on |
|
Starting build on |
| return; | ||
| } | ||
| } | ||
| fClassInfo = gInterpreter->ClassInfo_Factory(givenInfo); |
There was a problem hiding this comment.
These two are the only lines about which we did not reach consensus during the PPP meeting today. The open question is whether we need to put them in a critical section or if it is enough to take the lock in LoadClassInfo.
|
Starting build on |
|
Build failed on centos7/gcc49. |
core/meta/src/TClass.cxx
Outdated
| } | ||
| } | ||
|
|
||
| R__LOCKGUARD(gInterpreterMutex); |
There was a problem hiding this comment.
Note that taking the lock inside TClass::Init is (should be) redundant as Init must only be called from the constructors and they all (should) take the lock before calling Init.
There was a problem hiding this comment.
except for this minor comment. the PR looks good.
There was a problem hiding this comment.
Ok, I will push an updated version. Thanks for reviewing.
There was a problem hiding this comment.
Done, I removed that lock. I will wait for Jenkins to run and then merge.
|
Starting build on |
|
Starting build on |
|
Starting build on |
|
Build failed on mac1012/native. Failing tests: |
The member fCanLoadClassInfo is used to check if the class info is available and/or is loaded. If it is already available at initialization, then there is no reason to call LoadClassInfo().
|
Starting build on |
|
Starting build on |
|
Merged. Clang-format wanted to put my |
|
Build failed on ubuntu14/native. |
|
Build failed on mac1012/native. Failing tests: |
4 participants
This is a refactored version of part of PR #709. The figure attached below shows an example of the difference in wait time, as measured by VTune for parallel filling of a TTree with random numbers. The number of waits on
LoadClassInfo()is reduced from 451 to just 8 (one wait per thread).