TTreeCache: start a new fill at the end of the previous one. #8049
Conversation
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
See cms-sw/cmssw#33361 Fixes root-project#8048 Now, set the start point of the filling to be the end of the previous filling rather than the start of the current cluster (which can sometimes be before the end of the previous filling) Issue: The error message was inaccurate, it did not take into account jagged filling of the TTreeCache. In this case, the cache was filled with a little more than one cluster and when it needs to do the next refill it restarted from the cluster start boundary of that partially downloaded cluster which is “indeed” within the range of the last TreeCache fill (i.e. the error). We did not see the problem with a local file because the TTreeCache usage is different. CMSSW take note of whether prefetching (asynchronous reads) is available for a while or not. In the setup CMSSW has, the prefetching (asynchronous reads) is available for the local file but not for the network/remote file. In addition when prefetching (asynchronous reads) is not available, CMSSW uses multiple TTreeCache for a given TTree while it uses only one when prefetching (asynchronous reads) is available. This results in the pattern of filling to be different between the 2 cases.
|
Starting build on |
|
Build failed on mac11.0/cxx17. Warnings:
And 1475 more Failing tests:
|
jblomer
requested changes
May 3, 2021
Contributor
jblomer
left a comment
jblomer
left a comment
There was a problem hiding this comment.
It would be nice to have a test for jagged filling
| if (showMore || gDebug > 6) | ||
| Info("FillBuffer", "Looking at cluster spanning from %lld to %lld", fEntryCurrent, fEntryNext); | ||
|
|
||
| if (0 <= fNextClusterStart && fEntryCurrent < fNextClusterStart && !(fEntryCurrent < fNextClusterStart)) |
Contributor
There was a problem hiding this comment.
fEntryCurrent < fNextClusterStart && !(fEntryCurrent < fNextClusterStart) doesn't seem to make sense.
Member
Author
There was a problem hiding this comment.
You are right. This was intended to detect when the user is moving backward but a "typo/thinko" mess it up :( rendering the whole PR moot ... i.e. Test need to go in at the same time...
Test Results 17 files 17 suites 3d 17h 45m 27s ⏱️ For more details on these failures, see this check. Results for commit 7da8b32. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
See cms-sw/cmssw#33361
Fixes #8048
Now, set the start point of the filling to be the end of the previous filling rather than the start of the current cluster (which can sometimes
be before the end of the previous filling)
Issue: The error message was inaccurate, it did not take into account jagged filling of the TTreeCache. In this case, the cache was filled with a little more than one cluster and when it needs to do the next refill it restarted from the cluster start boundary of that partially downloaded cluster which is “indeed” within the range of the last TreeCache fill (i.e. the error).
We did not see the problem with a local file because the TTreeCache usage is different. CMSSW take note of whether prefetching (asynchronous reads) is available for a while or not. In the setup CMSSW has, the prefetching (asynchronous reads) is available for the local file but not for the network/remote file. In addition when prefetching (asynchronous reads) is not available, CMSSW uses multiple TTreeCache for a given TTree while it uses only one when prefetching (asynchronous reads) is available. This results in the pattern of filling to be different between the 2 cases.