[ntuple] Enable shared access to RNTupleFileWriter#21593
Conversation
Test Results 21 files 21 suites 3d 15h 40m 7s ⏱️ For more details on these failures, see this check. Results for commit f7a7b61. ♻️ This comment has been updated with latest results. |
silverweed
force-pushed
the
minifile_shared
branch
2 times, most recently
from
4df8bec to
0e31934
Compare
silverweed
changed the title
silverweed
changed the title
silverweed
force-pushed
the
minifile_shared
branch
2 times, most recently
from
613f3e4 to
f2c8fde
Compare
jblomer
left a comment
jblomer
left a comment
There was a problem hiding this comment.
Thanks! Apologies for the huge reviewing delay!
silverweed
force-pushed
the
minifile_shared
branch
2 times, most recently
from
ccdccdc to
c7f4119
Compare
silverweed
force-pushed
the
minifile_shared
branch
from
c7f4119 to
f7a7b61
Compare
| /// Whether the C file stream has been opened with Direct I/O, introducing alignment requirements. | ||
| bool fDirectIO = false; |
There was a problem hiding this comment.
Sorry, didn't find the time before to look at this PR: what's the reason for leaving fDirectIO out? I think all clones must follow the same alignment procedure, or the offsets in the RSharedData might be wrong / inconsistent...
| new RNTupleFileWriter(ntupleName, fNTupleAnchor.GetMaxKeySize(), /*hidden=*/true)); | ||
| auto &clonedFile = std::get<RImplSimple>(writer->fFile); | ||
| clonedFile.fShared = file->fShared; | ||
| clonedFile.fDirectIO = file->fDirectIO; |
There was a problem hiding this comment.
Ah, I see you are also copying fDirectIO here, so the code is correct. But wouldn't it be easier to move the member into the shared data?
There was a problem hiding this comment.
I believe I wasn't sure whether we would want/need fDirectIO to be necessarily the same for all clones; if that is the case I can make a small PR to move it in as well
There was a problem hiding this comment.
Yes, it must be the same for all clones that share the FILE *
3 participants
Allow for multiple RNTupleFileWriters to share a single underlying Simple file. This is required for attribute writing support in Minifile-backed RNTuples.
The idea is turning all shared properties of RImplSimple into a
shared_ptr-accessed struct which is referenced by all clones of a Writer. Access is strictly single-threaded but that's enough for attribute writing.