From 130463acd452210d991128eb7306e1318b112adf Mon Sep 17 00:00:00 2001 From: silverweed Date: Thu, 12 Mar 2026 17:39:38 +0100 Subject: [PATCH 1/2] [ntuple] Introduce RImplSimple::RSharedData --- tree/ntuple/inc/ROOT/RMiniFile.hxx | 39 +++--- tree/ntuple/src/RMiniFile.cxx | 216 ++++++++++++++++------------- 2 files changed, 142 insertions(+), 113 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RMiniFile.hxx b/tree/ntuple/inc/ROOT/RMiniFile.hxx index 972af4b4e16a6..e8c3feaf62b9d 100644 --- a/tree/ntuple/inc/ROOT/RMiniFile.hxx +++ b/tree/ntuple/inc/ROOT/RMiniFile.hxx @@ -157,29 +157,36 @@ private: /// aligned to kBlockAlign... static constexpr std::size_t kHeaderBlockSize = 4096; - // fHeaderBlock and fBlock are raw pointers because we have to manually call operator new and delete. - unsigned char *fHeaderBlock = nullptr; - std::size_t fBlockSize = 0; - std::uint64_t fBlockOffset = 0; - unsigned char *fBlock = nullptr; - - /// For the simplest cases, a C file stream can be used for writing - FILE *fFile = nullptr; /// Whether the C file stream has been opened with Direct I/O, introducing alignment requirements. bool fDirectIO = false; - /// Keeps track of the seek offset - std::uint64_t fFilePos = 0; - /// Keeps track of the next key offset - std::uint64_t fKeyOffset = 0; - /// Keeps track of TFile control structures, which need to be updated on committing the data set - std::unique_ptr fControlBlock; + + struct RSharedData { + /// For the simplest cases, a C file stream can be used for writing + FILE *fFile = nullptr; + /// Keeps track of the seek offset + std::uint64_t fFilePos = 0; + /// Keeps track of the next key offset + std::uint64_t fKeyOffset = 0; + + // fHeaderBlock and fBlock are raw pointers because we have to manually call operator new and delete. + unsigned char *fHeaderBlock = nullptr; + std::size_t fBlockSize = 0; + std::uint64_t fBlockOffset = 0; + unsigned char *fBlock = nullptr; + + /// Keeps track of TFile control structures, which need to be updated on committing the data set + std::unique_ptr fControlBlock; + + RSharedData(FILE *file); + ~RSharedData(); + }; + std::shared_ptr fShared; RImplSimple(); RImplSimple(const RImplSimple &other) = delete; RImplSimple(RImplSimple &&other) = delete; RImplSimple &operator=(const RImplSimple &other) = delete; RImplSimple &operator=(RImplSimple &&other) = delete; - ~RImplSimple(); void AllocateBuffers(std::size_t bufferSize); void Flush(); @@ -195,7 +202,7 @@ private: /// it must be written *before* the returned offset. (Note that the array type is purely documentation, the /// argument is actually just a pointer.) std::uint64_t ReserveBlobKey(std::size_t nbytes, std::size_t len, unsigned char keyBuffer[kBlobKeyLen] = nullptr); - operator bool() const { return fFile; } + operator bool() const { return fShared->fFile; } }; template diff --git a/tree/ntuple/src/RMiniFile.cxx b/tree/ntuple/src/RMiniFile.cxx index 59570dee2208b..1e70ef505872a 100644 --- a/tree/ntuple/src/RMiniFile.cxx +++ b/tree/ntuple/src/RMiniFile.cxx @@ -986,16 +986,24 @@ void ROOT::Internal::RNTupleFileWriter::RImplSimple::AllocateBuffers(std::size_t static_assert(kHeaderBlockSize % kBlockAlign == 0, "invalid header block size"); if (bufferSize % kBlockAlign != 0) throw RException(R__FAIL("Buffer size not a multiple of alignment: " + std::to_string(bufferSize))); - fBlockSize = bufferSize; + + assert(!fShared); + + fShared = std::make_shared(nullptr); + + fShared->fBlockSize = bufferSize; + fShared->fControlBlock = std::make_unique(); std::align_val_t blockAlign{kBlockAlign}; - fHeaderBlock = static_cast(::operator new[](kHeaderBlockSize, blockAlign)); - memset(fHeaderBlock, 0, kHeaderBlockSize); - fBlock = static_cast(::operator new[](fBlockSize, blockAlign)); - memset(fBlock, 0, fBlockSize); + fShared->fHeaderBlock = static_cast(::operator new[](kHeaderBlockSize, blockAlign)); + memset(fShared->fHeaderBlock, 0, kHeaderBlockSize); + fShared->fBlock = static_cast(::operator new[](fShared->fBlockSize, blockAlign)); + memset(fShared->fBlock, 0, fShared->fBlockSize); } -ROOT::Internal::RNTupleFileWriter::RImplSimple::~RImplSimple() +ROOT::Internal::RNTupleFileWriter::RImplSimple::RSharedData::RSharedData(FILE *file) : fFile(file) {} + +ROOT::Internal::RNTupleFileWriter::RImplSimple::RSharedData::~RSharedData() { if (fFile) fclose(fFile); @@ -1020,93 +1028,97 @@ int FSeek64(FILE *stream, std::int64_t offset, int origin) void ROOT::Internal::RNTupleFileWriter::RImplSimple::Flush() { + auto &shared = *fShared; + // Write the last partially filled block, which may still need appropriate alignment for Direct I/O. // If it is the first block, get the updated header block. - if (fBlockOffset == 0) { + if (shared.fBlockOffset == 0) { std::size_t headerBlockSize = kHeaderBlockSize; - if (headerBlockSize > fFilePos) { - headerBlockSize = fFilePos; + if (headerBlockSize > shared.fFilePos) { + headerBlockSize = shared.fFilePos; } - memcpy(fBlock, fHeaderBlock, headerBlockSize); + memcpy(shared.fBlock, shared.fHeaderBlock, headerBlockSize); } - std::size_t retval = FSeek64(fFile, fBlockOffset, SEEK_SET); + std::size_t retval = FSeek64(shared.fFile, shared.fBlockOffset, SEEK_SET); if (retval) throw RException(R__FAIL(std::string("Seek failed: ") + strerror(errno))); - std::size_t lastBlockSize = fFilePos - fBlockOffset; - R__ASSERT(lastBlockSize <= fBlockSize); + std::size_t lastBlockSize = shared.fFilePos - shared.fBlockOffset; + R__ASSERT(lastBlockSize <= shared.fBlockSize); if (fDirectIO) { // Round up to a multiple of kBlockAlign. lastBlockSize += kBlockAlign - 1; lastBlockSize = (lastBlockSize / kBlockAlign) * kBlockAlign; - R__ASSERT(lastBlockSize <= fBlockSize); + R__ASSERT(lastBlockSize <= shared.fBlockSize); } - retval = fwrite(fBlock, 1, lastBlockSize, fFile); + retval = fwrite(shared.fBlock, 1, lastBlockSize, shared.fFile); if (retval != lastBlockSize) throw RException(R__FAIL(std::string("write failed: ") + strerror(errno))); // Write the (updated) header block, unless it was part of the write above. - if (fBlockOffset > 0) { - retval = FSeek64(fFile, 0, SEEK_SET); + if (shared.fBlockOffset > 0) { + retval = FSeek64(shared.fFile, 0, SEEK_SET); if (retval) throw RException(R__FAIL(std::string("Seek failed: ") + strerror(errno))); - retval = fwrite(fHeaderBlock, 1, kHeaderBlockSize, fFile); + retval = fwrite(shared.fHeaderBlock, 1, kHeaderBlockSize, shared.fFile); if (retval != RImplSimple::kHeaderBlockSize) throw RException(R__FAIL(std::string("write failed: ") + strerror(errno))); } - retval = fflush(fFile); + retval = fflush(shared.fFile); if (retval) throw RException(R__FAIL(std::string("Flush failed: ") + strerror(errno))); } void ROOT::Internal::RNTupleFileWriter::RImplSimple::Write(const void *buffer, size_t nbytes, std::int64_t offset) { - R__ASSERT(fFile); + auto &shared = *fShared; + + R__ASSERT(shared.fFile); size_t retval; - if ((offset >= 0) && (static_cast(offset) != fFilePos)) { - fFilePos = offset; + if ((offset >= 0) && (static_cast(offset) != shared.fFilePos)) { + shared.fFilePos = offset; } // Keep header block to overwrite on commit. - if (fFilePos < kHeaderBlockSize) { + if (shared.fFilePos < kHeaderBlockSize) { std::size_t headerBytes = nbytes; - if (fFilePos + headerBytes > kHeaderBlockSize) { - headerBytes = kHeaderBlockSize - fFilePos; + if (shared.fFilePos + headerBytes > kHeaderBlockSize) { + headerBytes = kHeaderBlockSize - shared.fFilePos; } - memcpy(fHeaderBlock + fFilePos, buffer, headerBytes); + memcpy(shared.fHeaderBlock + shared.fFilePos, buffer, headerBytes); } - R__ASSERT(fFilePos >= fBlockOffset); + R__ASSERT(shared.fFilePos >= shared.fBlockOffset); while (nbytes > 0) { - std::uint64_t posInBlock = fFilePos % fBlockSize; - std::uint64_t blockOffset = fFilePos - posInBlock; - if (blockOffset != fBlockOffset) { + std::uint64_t posInBlock = shared.fFilePos % shared.fBlockSize; + std::uint64_t blockOffset = shared.fFilePos - posInBlock; + if (blockOffset != shared.fBlockOffset) { // Write the block. - retval = FSeek64(fFile, fBlockOffset, SEEK_SET); + retval = FSeek64(shared.fFile, shared.fBlockOffset, SEEK_SET); if (retval) throw RException(R__FAIL(std::string("Seek failed: ") + strerror(errno))); - retval = fwrite(fBlock, 1, fBlockSize, fFile); - if (retval != fBlockSize) + retval = fwrite(shared.fBlock, 1, shared.fBlockSize, shared.fFile); + if (retval != shared.fBlockSize) throw RException(R__FAIL(std::string("write failed: ") + strerror(errno))); // Null the buffer contents for good measure. - memset(fBlock, 0, fBlockSize); + memset(shared.fBlock, 0, shared.fBlockSize); } - fBlockOffset = blockOffset; + shared.fBlockOffset = blockOffset; std::size_t blockSize = nbytes; - if (blockSize > fBlockSize - posInBlock) { - blockSize = fBlockSize - posInBlock; + if (blockSize > shared.fBlockSize - posInBlock) { + blockSize = shared.fBlockSize - posInBlock; } - memcpy(fBlock + posInBlock, buffer, blockSize); + memcpy(shared.fBlock + posInBlock, buffer, blockSize); buffer = static_cast(buffer) + blockSize; nbytes -= blockSize; - fFilePos += blockSize; + shared.fFilePos += blockSize; } } @@ -1116,20 +1128,22 @@ ROOT::Internal::RNTupleFileWriter::RImplSimple::WriteKey(const void *buffer, std const std::string &className, const std::string &objectName, const std::string &title) { + auto &shared = *fShared; + if (offset > 0) - fKeyOffset = offset; + shared.fKeyOffset = offset; RTFString strClass{className}; RTFString strObject{objectName}; RTFString strTitle{title}; - RTFKey key(fKeyOffset, directoryOffset, strClass, strObject, strTitle, len, nbytes); - Write(&key, key.GetHeaderSize(), fKeyOffset); + RTFKey key(shared.fKeyOffset, directoryOffset, strClass, strObject, strTitle, len, nbytes); + Write(&key, key.GetHeaderSize(), shared.fKeyOffset); Write(&strClass, strClass.GetSize()); Write(&strObject, strObject.GetSize()); Write(&strTitle, strTitle.GetSize()); - auto offsetData = fFilePos; + auto offsetData = shared.fFilePos; // The next key starts after the data. - fKeyOffset = offsetData + nbytes; + shared.fKeyOffset = offsetData + nbytes; if (buffer) Write(buffer, nbytes); @@ -1139,17 +1153,19 @@ ROOT::Internal::RNTupleFileWriter::RImplSimple::WriteKey(const void *buffer, std std::uint64_t ROOT::Internal::RNTupleFileWriter::RImplSimple::ReserveBlobKey(std::size_t nbytes, std::size_t len, unsigned char keyBuffer[kBlobKeyLen]) { + auto &shared = *fShared; + if (keyBuffer) { - PrepareBlobKey(fKeyOffset, nbytes, len, keyBuffer); + PrepareBlobKey(shared.fKeyOffset, nbytes, len, keyBuffer); } else { unsigned char localKeyBuffer[kBlobKeyLen]; - PrepareBlobKey(fKeyOffset, nbytes, len, localKeyBuffer); - Write(localKeyBuffer, kBlobKeyLen, fKeyOffset); + PrepareBlobKey(shared.fKeyOffset, nbytes, len, localKeyBuffer); + Write(localKeyBuffer, kBlobKeyLen, shared.fKeyOffset); } - auto offsetData = fKeyOffset + kBlobKeyLen; + auto offsetData = shared.fKeyOffset + kBlobKeyLen; // The next key starts after the data. - fKeyOffset = offsetData + nbytes; + shared.fKeyOffset = offsetData + nbytes; return offsetData; } @@ -1225,8 +1241,7 @@ std::uint64_t ROOT::Internal::RNTupleFileWriter::RImplRFile::ReserveBlobKey(size ROOT::Internal::RNTupleFileWriter::RNTupleFileWriter(std::string_view name, std::uint64_t maxKeySize) : fNTupleName(name) { - auto &fileSimple = fFile.emplace(); - fileSimple.fControlBlock = std::make_unique(); + fFile.emplace(); fNTupleAnchor.fMaxKeySize = maxKeySize; auto infoRNTuple = RNTuple::Class()->GetStreamerInfo(); fStreamerInfoMap[infoRNTuple->GetNumber()] = infoRNTuple; @@ -1272,9 +1287,9 @@ ROOT::Internal::RNTupleFileWriter::Recreate(std::string_view ntupleName, std::st auto writer = std::unique_ptr(new RNTupleFileWriter(ntupleName, options.GetMaxKeySize())); RImplSimple &fileSimple = std::get(writer->fFile); - fileSimple.fFile = fileStream; fileSimple.fDirectIO = options.GetUseDirectIO(); fileSimple.AllocateBuffers(options.GetWriteBufferSize()); + fileSimple.fShared->fFile = fileStream; writer->fFileName = fileName; int defaultCompression = options.GetCompression(); @@ -1333,8 +1348,8 @@ void ROOT::Internal::RNTupleFileWriter::Seek(std::uint64_t offset) if (!fileSimple) throw RException(R__FAIL("invalid attempt to seek non-simple writer")); - fileSimple->fFilePos = offset; - fileSimple->fKeyOffset = offset; + fileSimple->fShared->fFilePos = offset; + fileSimple->fShared->fKeyOffset = offset; // The next Write() will Flush() if necessary. } @@ -1374,9 +1389,10 @@ void ROOT::Internal::RNTupleFileWriter::Commit(int compression) RTFNTuple ntupleOnDisk(fNTupleAnchor); // Compute the checksum std::uint64_t checksum = XXH3_64bits(ntupleOnDisk.GetPtrCkData(), ntupleOnDisk.GetSizeCkData()); - memcpy(fileSimple.fHeaderBlock + fileSimple.fControlBlock->fSeekNTuple, &ntupleOnDisk, ntupleOnDisk.GetSize()); - memcpy(fileSimple.fHeaderBlock + fileSimple.fControlBlock->fSeekNTuple + ntupleOnDisk.GetSize(), &checksum, - sizeof(checksum)); + memcpy(fileSimple.fShared->fHeaderBlock + fileSimple.fShared->fControlBlock->fSeekNTuple, &ntupleOnDisk, + ntupleOnDisk.GetSize()); + memcpy(fileSimple.fShared->fHeaderBlock + fileSimple.fShared->fControlBlock->fSeekNTuple + ntupleOnDisk.GetSize(), + &checksum, sizeof(checksum)); fileSimple.Flush(); return; } @@ -1387,11 +1403,13 @@ void ROOT::Internal::RNTupleFileWriter::Commit(int compression) WriteTFileFreeList(); // NOTE: this is written uncompressed // Update header and TFile record - memcpy(fileSimple.fHeaderBlock, &fileSimple.fControlBlock->fHeader, fileSimple.fControlBlock->fHeader.GetSize()); - R__ASSERT(fileSimple.fControlBlock->fSeekFileRecord + fileSimple.fControlBlock->fFileRecord.GetSize() < + memcpy(fileSimple.fShared->fHeaderBlock, &fileSimple.fShared->fControlBlock->fHeader, + fileSimple.fShared->fControlBlock->fHeader.GetSize()); + R__ASSERT(fileSimple.fShared->fControlBlock->fSeekFileRecord + + fileSimple.fShared->fControlBlock->fFileRecord.GetSize() < RImplSimple::kHeaderBlockSize); - memcpy(fileSimple.fHeaderBlock + fileSimple.fControlBlock->fSeekFileRecord, &fileSimple.fControlBlock->fFileRecord, - fileSimple.fControlBlock->fFileRecord.GetSize()); + memcpy(fileSimple.fShared->fHeaderBlock + fileSimple.fShared->fControlBlock->fSeekFileRecord, + &fileSimple.fShared->fControlBlock->fFileRecord, fileSimple.fShared->fControlBlock->fFileRecord.GetSize()); fileSimple.Flush(); } @@ -1470,8 +1488,8 @@ ROOT::Internal::RNTupleFileWriter::ReserveBlob(size_t nbytes, size_t len, unsign std::uint64_t offset; if (auto *fileSimple = std::get_if(&fFile)) { if (fIsBare) { - offset = fileSimple->fKeyOffset; - fileSimple->fKeyOffset += nbytes; + offset = fileSimple->fShared->fKeyOffset; + fileSimple->fShared->fKeyOffset += nbytes; } else { offset = fileSimple->ReserveBlobKey(nbytes, len, keyBuffer); } @@ -1525,11 +1543,11 @@ void ROOT::Internal::RNTupleFileWriter::WriteBareFileSkeleton(int defaultCompres // Write zero-initialized ntuple to reserve the space; will be overwritten on commit RTFNTuple ntupleOnDisk; - fileSimple.fControlBlock->fSeekNTuple = fileSimple.fFilePos; + fileSimple.fShared->fControlBlock->fSeekNTuple = fileSimple.fShared->fFilePos; fileSimple.Write(&ntupleOnDisk, ntupleOnDisk.GetSize()); std::uint64_t checksum = 0; fileSimple.Write(&checksum, sizeof(checksum)); - fileSimple.fKeyOffset = fileSimple.fFilePos; + fileSimple.fShared->fKeyOffset = fileSimple.fShared->fFilePos; } void ROOT::Internal::RNTupleFileWriter::WriteTFileStreamerInfo(int compression) @@ -1554,9 +1572,9 @@ void ROOT::Internal::RNTupleFileWriter::WriteTFileStreamerInfo(int compression) RTFString strStreamerInfo{"StreamerInfo"}; RTFString strStreamerTitle{"Doubly linked list"}; auto &fileSimple = std::get(fFile); - fileSimple.fControlBlock->fHeader.SetSeekInfo(fileSimple.fKeyOffset); - auto keyLen = RTFKey(fileSimple.fControlBlock->fHeader.GetSeekInfo(), RTFHeader::kBEGIN, strTList, strStreamerInfo, - strStreamerTitle, 0) + fileSimple.fShared->fControlBlock->fHeader.SetSeekInfo(fileSimple.fShared->fKeyOffset); + auto keyLen = RTFKey(fileSimple.fShared->fControlBlock->fHeader.GetSeekInfo(), RTFHeader::kBEGIN, strTList, + strStreamerInfo, strStreamerTitle, 0) .fKeyLen; TBufferFile buffer(TBuffer::kWrite, keyLen + 1); @@ -1570,10 +1588,10 @@ void ROOT::Internal::RNTupleFileWriter::WriteTFileStreamerInfo(int compression) auto szZipStreamerInfos = RNTupleCompressor::Zip(bufPayload, lenPayload, compression, zipStreamerInfos.get()); fileSimple.WriteKey(zipStreamerInfos.get(), szZipStreamerInfos, lenPayload, - fileSimple.fControlBlock->fHeader.GetSeekInfo(), RTFHeader::kBEGIN, "TList", "StreamerInfo", - "Doubly linked list"); - fileSimple.fControlBlock->fHeader.SetNbytesInfo(fileSimple.fFilePos - - fileSimple.fControlBlock->fHeader.GetSeekInfo()); + fileSimple.fShared->fControlBlock->fHeader.GetSeekInfo(), RTFHeader::kBEGIN, "TList", + "StreamerInfo", "Doubly linked list"); + fileSimple.fShared->fControlBlock->fHeader.SetNbytesInfo(fileSimple.fShared->fFilePos - + fileSimple.fShared->fControlBlock->fHeader.GetSeekInfo()); } void ROOT::Internal::RNTupleFileWriter::WriteTFileKeysList(std::uint64_t anchorSize) @@ -1584,14 +1602,16 @@ void ROOT::Internal::RNTupleFileWriter::WriteTFileKeysList(std::uint64_t anchorS RTFString strFileName{fFileName}; auto &fileSimple = std::get(fFile); - RTFKey keyRNTuple(fileSimple.fControlBlock->fSeekNTuple, RTFHeader::kBEGIN, strRNTupleClass, strRNTupleName, + RTFKey keyRNTuple(fileSimple.fShared->fControlBlock->fSeekNTuple, RTFHeader::kBEGIN, strRNTupleClass, strRNTupleName, strEmpty, RTFNTuple::GetSizePlusChecksum(), anchorSize); - fileSimple.fControlBlock->fFileRecord.SetSeekKeys(fileSimple.fKeyOffset); + auto &fileShared = *fileSimple.fShared; + fileSimple.fShared->fControlBlock->fFileRecord.SetSeekKeys(fileShared.fKeyOffset); RTFKeyList keyList{1}; - RTFKey keyKeyList(fileSimple.fControlBlock->fFileRecord.GetSeekKeys(), RTFHeader::kBEGIN, strEmpty, strFileName, - strEmpty, keyList.GetSize() + keyRNTuple.fKeyLen); - fileSimple.Write(&keyKeyList, keyKeyList.GetHeaderSize(), fileSimple.fControlBlock->fFileRecord.GetSeekKeys()); + RTFKey keyKeyList(fileSimple.fShared->fControlBlock->fFileRecord.GetSeekKeys(), RTFHeader::kBEGIN, strEmpty, + strFileName, strEmpty, keyList.GetSize() + keyRNTuple.fKeyLen); + fileSimple.Write(&keyKeyList, keyKeyList.GetHeaderSize(), + fileSimple.fShared->fControlBlock->fFileRecord.GetSeekKeys()); fileSimple.Write(&strEmpty, strEmpty.GetSize()); fileSimple.Write(&strFileName, strFileName.GetSize()); fileSimple.Write(&strEmpty, strEmpty.GetSize()); @@ -1601,27 +1621,28 @@ void ROOT::Internal::RNTupleFileWriter::WriteTFileKeysList(std::uint64_t anchorS fileSimple.Write(&strRNTupleClass, strRNTupleClass.GetSize()); fileSimple.Write(&strRNTupleName, strRNTupleName.GetSize()); fileSimple.Write(&strEmpty, strEmpty.GetSize()); - fileSimple.fControlBlock->fFileRecord.fNBytesKeys = - fileSimple.fFilePos - fileSimple.fControlBlock->fFileRecord.GetSeekKeys(); - fileSimple.fKeyOffset = fileSimple.fFilePos; + fileSimple.fShared->fControlBlock->fFileRecord.fNBytesKeys = + fileShared.fFilePos - fileSimple.fShared->fControlBlock->fFileRecord.GetSeekKeys(); + fileShared.fKeyOffset = fileShared.fFilePos; } void ROOT::Internal::RNTupleFileWriter::WriteTFileFreeList() { auto &fileSimple = std::get(fFile); - fileSimple.fControlBlock->fHeader.SetSeekFree(fileSimple.fKeyOffset); + auto &fileShared = *fileSimple.fShared; + fileSimple.fShared->fControlBlock->fHeader.SetSeekFree(fileShared.fKeyOffset); RTFString strEmpty; RTFString strFileName{fFileName}; RTFFreeEntry freeEntry; - RTFKey keyFreeList(fileSimple.fControlBlock->fHeader.GetSeekFree(), RTFHeader::kBEGIN, strEmpty, strFileName, - strEmpty, freeEntry.GetSize()); - std::uint64_t firstFree = fileSimple.fControlBlock->fHeader.GetSeekFree() + keyFreeList.GetSize(); + RTFKey keyFreeList(fileSimple.fShared->fControlBlock->fHeader.GetSeekFree(), RTFHeader::kBEGIN, strEmpty, + strFileName, strEmpty, freeEntry.GetSize()); + std::uint64_t firstFree = fileSimple.fShared->fControlBlock->fHeader.GetSeekFree() + keyFreeList.GetSize(); freeEntry.Set(firstFree, std::max(2000000000ULL, ((firstFree / 1000000000ULL) + 1) * 1000000000ULL)); fileSimple.WriteKey(&freeEntry, freeEntry.GetSize(), freeEntry.GetSize(), - fileSimple.fControlBlock->fHeader.GetSeekFree(), RTFHeader::kBEGIN, "", fFileName, ""); - fileSimple.fControlBlock->fHeader.SetNbytesFree(fileSimple.fFilePos - - fileSimple.fControlBlock->fHeader.GetSeekFree()); - fileSimple.fControlBlock->fHeader.SetEnd(fileSimple.fFilePos); + fileSimple.fShared->fControlBlock->fHeader.GetSeekFree(), RTFHeader::kBEGIN, "", fFileName, ""); + fileSimple.fShared->fControlBlock->fHeader.SetNbytesFree(fileShared.fFilePos - + fileSimple.fShared->fControlBlock->fHeader.GetSeekFree()); + fileSimple.fShared->fControlBlock->fHeader.SetEnd(fileShared.fFilePos); } std::uint64_t ROOT::Internal::RNTupleFileWriter::WriteTFileNTupleKey(int compression) @@ -1633,7 +1654,7 @@ std::uint64_t ROOT::Internal::RNTupleFileWriter::WriteTFileNTupleKey(int compres RTFNTuple ntupleOnDisk(fNTupleAnchor); RUInt64BE checksum{XXH3_64bits(ntupleOnDisk.GetPtrCkData(), ntupleOnDisk.GetSizeCkData())}; auto &fileSimple = std::get(fFile); - fileSimple.fControlBlock->fSeekNTuple = fileSimple.fKeyOffset; + fileSimple.fShared->fControlBlock->fSeekNTuple = fileSimple.fShared->fKeyOffset; char keyBuf[RTFNTuple::GetSizePlusChecksum()]; @@ -1645,8 +1666,8 @@ std::uint64_t ROOT::Internal::RNTupleFileWriter::WriteTFileNTupleKey(int compres char zipAnchor[RTFNTuple::GetSizePlusChecksum()]; auto szZipAnchor = RNTupleCompressor::Zip(keyBuf, sizeAnchor, compression, zipAnchor); - fileSimple.WriteKey(zipAnchor, szZipAnchor, sizeof(keyBuf), fileSimple.fControlBlock->fSeekNTuple, RTFHeader::kBEGIN, - "ROOT::RNTuple", fNTupleName, ""); + fileSimple.WriteKey(zipAnchor, szZipAnchor, sizeof(keyBuf), fileSimple.fShared->fControlBlock->fSeekNTuple, + RTFHeader::kBEGIN, "ROOT::RNTuple", fNTupleName, ""); return szZipAnchor; } @@ -1657,7 +1678,7 @@ void ROOT::Internal::RNTupleFileWriter::WriteTFileSkeleton(int defaultCompressio RTFString strEmpty; auto &fileSimple = std::get(fFile); - fileSimple.fControlBlock->fHeader = RTFHeader(defaultCompression); + fileSimple.fShared->fControlBlock->fHeader = RTFHeader(defaultCompression); RTFUUID uuid; @@ -1665,8 +1686,8 @@ void ROOT::Internal::RNTupleFileWriter::WriteTFileSkeleton(int defaultCompressio RTFKey keyRoot(RTFHeader::kBEGIN, 0, strTFile, strFileName, strEmpty, sizeof(RTFDirectory) + strFileName.GetSize() + strEmpty.GetSize() + uuid.GetSize()); std::uint32_t nbytesName = keyRoot.fKeyLen + strFileName.GetSize() + 1; - fileSimple.fControlBlock->fFileRecord.fNBytesName = nbytesName; - fileSimple.fControlBlock->fHeader.SetNbytesName(nbytesName); + fileSimple.fShared->fControlBlock->fFileRecord.fNBytesName = nbytesName; + fileSimple.fShared->fControlBlock->fHeader.SetNbytesName(nbytesName); fileSimple.Write(&keyRoot, keyRoot.GetHeaderSize(), RTFHeader::kBEGIN); // Write class name, object name, and title for the TFile key. @@ -1677,13 +1698,14 @@ void ROOT::Internal::RNTupleFileWriter::WriteTFileSkeleton(int defaultCompressio fileSimple.Write(&strFileName, strFileName.GetSize()); fileSimple.Write(&strEmpty, strEmpty.GetSize()); // Will be overwritten on commit - fileSimple.fControlBlock->fSeekFileRecord = fileSimple.fFilePos; - fileSimple.Write(&fileSimple.fControlBlock->fFileRecord, fileSimple.fControlBlock->fFileRecord.GetSize()); + fileSimple.fShared->fControlBlock->fSeekFileRecord = fileSimple.fShared->fFilePos; + fileSimple.Write(&fileSimple.fShared->fControlBlock->fFileRecord, + fileSimple.fShared->fControlBlock->fFileRecord.GetSize()); fileSimple.Write(&uuid, uuid.GetSize()); // Padding bytes to allow the TFile record to grow for a big file RUInt32BE padding{0}; for (int i = 0; i < 3; ++i) fileSimple.Write(&padding, sizeof(padding)); - fileSimple.fKeyOffset = fileSimple.fFilePos; + fileSimple.fShared->fKeyOffset = fileSimple.fShared->fFilePos; } From f2c8fde91ad2811afb0852e52ccee62f543c233e Mon Sep 17 00:00:00 2001 From: silverweed Date: Thu, 12 Mar 2026 17:47:26 +0100 Subject: [PATCH 2/2] [ntuple] add cloning for RImplSimple-based writer --- tree/ntuple/inc/ROOT/RMiniFile.hxx | 1 + tree/ntuple/src/RMiniFile.cxx | 49 +++++++++++++++++++----------- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RMiniFile.hxx b/tree/ntuple/inc/ROOT/RMiniFile.hxx index e8c3feaf62b9d..9405fe4c38231 100644 --- a/tree/ntuple/inc/ROOT/RMiniFile.hxx +++ b/tree/ntuple/inc/ROOT/RMiniFile.hxx @@ -159,6 +159,7 @@ private: /// Whether the C file stream has been opened with Direct I/O, introducing alignment requirements. bool fDirectIO = false; + bool fIsClone = false; struct RSharedData { /// For the simplest cases, a C file stream can be used for writing diff --git a/tree/ntuple/src/RMiniFile.cxx b/tree/ntuple/src/RMiniFile.cxx index 1e70ef505872a..4b8b5133dfae5 100644 --- a/tree/ntuple/src/RMiniFile.cxx +++ b/tree/ntuple/src/RMiniFile.cxx @@ -1335,11 +1335,22 @@ ROOT::Internal::RNTupleFileWriter::Append(std::string_view ntupleName, ROOT::Exp std::unique_ptr ROOT::Internal::RNTupleFileWriter::CloneWithDifferentName(std::string_view ntupleName) const { - if (auto *file = std::get_if(&fFile)) { - return Append(ntupleName, *file->fDirectory, fNTupleAnchor.fMaxKeySize); - } - // TODO: support also non-TFile-based writers - throw ROOT::RException(R__FAIL("cannot clone a non-TFile-based RNTupleFileWriter.")); + if (auto *tfile = std::get_if(&fFile)) { + return Append(ntupleName, *tfile->fDirectory, fNTupleAnchor.fMaxKeySize); + } else if (auto *file = std::get_if(&fFile)) { + if (fIsBare) + throw ROOT::RException(R__FAIL("cloning a bare file is currently unsupported")); + + auto writer = + std::unique_ptr(new RNTupleFileWriter(ntupleName, fNTupleAnchor.GetMaxKeySize())); + auto &clonedFile = std::get(writer->fFile); + clonedFile.fShared = file->fShared; + clonedFile.fDirectIO = file->fDirectIO; + clonedFile.fIsClone = true; + return writer; + } + // TODO: support also RFile-based writers + throw ROOT::RException(R__FAIL("cannot clone an RFile-based RNTupleFileWriter.")); } void ROOT::Internal::RNTupleFileWriter::Seek(std::uint64_t offset) @@ -1384,15 +1395,17 @@ void ROOT::Internal::RNTupleFileWriter::Commit(int compression) // Writing by C file stream: prepare the container format header and stream the RNTuple anchor object auto &fileSimple = std::get(fFile); + const auto &controlBlock = fileSimple.fShared->fControlBlock; + unsigned char *headerBlock = fileSimple.fShared->fHeaderBlock; if (fIsBare) { + assert(!fileSimple.fIsClone); + RTFNTuple ntupleOnDisk(fNTupleAnchor); // Compute the checksum std::uint64_t checksum = XXH3_64bits(ntupleOnDisk.GetPtrCkData(), ntupleOnDisk.GetSizeCkData()); - memcpy(fileSimple.fShared->fHeaderBlock + fileSimple.fShared->fControlBlock->fSeekNTuple, &ntupleOnDisk, - ntupleOnDisk.GetSize()); - memcpy(fileSimple.fShared->fHeaderBlock + fileSimple.fShared->fControlBlock->fSeekNTuple + ntupleOnDisk.GetSize(), - &checksum, sizeof(checksum)); + memcpy(headerBlock + controlBlock->fSeekNTuple, &ntupleOnDisk, ntupleOnDisk.GetSize()); + memcpy(headerBlock + controlBlock->fSeekNTuple + ntupleOnDisk.GetSize(), &checksum, sizeof(checksum)); fileSimple.Flush(); return; } @@ -1403,15 +1416,15 @@ void ROOT::Internal::RNTupleFileWriter::Commit(int compression) WriteTFileFreeList(); // NOTE: this is written uncompressed // Update header and TFile record - memcpy(fileSimple.fShared->fHeaderBlock, &fileSimple.fShared->fControlBlock->fHeader, - fileSimple.fShared->fControlBlock->fHeader.GetSize()); - R__ASSERT(fileSimple.fShared->fControlBlock->fSeekFileRecord + - fileSimple.fShared->fControlBlock->fFileRecord.GetSize() < - RImplSimple::kHeaderBlockSize); - memcpy(fileSimple.fShared->fHeaderBlock + fileSimple.fShared->fControlBlock->fSeekFileRecord, - &fileSimple.fShared->fControlBlock->fFileRecord, fileSimple.fShared->fControlBlock->fFileRecord.GetSize()); - - fileSimple.Flush(); + if (!fileSimple.fIsClone) { + memcpy(headerBlock, &controlBlock->fHeader, controlBlock->fHeader.GetSize()); + + R__ASSERT(controlBlock->fSeekFileRecord + controlBlock->fFileRecord.GetSize() < RImplSimple::kHeaderBlockSize); + memcpy(headerBlock + controlBlock->fSeekFileRecord, &controlBlock->fFileRecord, + controlBlock->fFileRecord.GetSize()); + + fileSimple.Flush(); + } } std::uint64_t ROOT::Internal::RNTupleFileWriter::WriteBlob(const void *data, size_t nbytes, size_t len)