diff --git a/io/io/inc/TFile.h b/io/io/inc/TFile.h index ee87f2bb96bb4..2cd0e29e02a99 100644 --- a/io/io/inc/TFile.h +++ b/io/io/inc/TFile.h @@ -276,6 +276,9 @@ class TFile : public TDirectoryFile { }; enum ERelativeTo { kBeg = 0, kCur = 1, kEnd = 2 }; enum { kStartBigFile = 2000000000 }; + enum { + kMaxGapSize = 2000000000 + }; // Maximum absolute value of the free segment on-disk size marker /// File type enum EFileType { // clang++ + class TBrowser; class TDirectory; class TFile; @@ -38,6 +40,11 @@ class TKey : public TNamed { Int_t UnzipBuffer(char *targetBuffer, const char *compressedBuffer) const; protected: + // After a key that has been placed in a gap larger than the key itself, one or very rarely two marker bytes + // follow to restore the linked list of segments. The two char buffers are meant to be written to disk, i.e. + // they should contain big-endian encoded negative integer values for the size of a gap, or zero if unused. + using GapHeaderBuf_t = std::array, 2>; + Int_t fVersion; ///< Key version identifier Int_t fNbytes; ///< Number of bytes for the whole key on file (key header and data) Int_t fObjlen; ///< Length of uncompressed object in bytes @@ -47,7 +54,7 @@ class TKey : public TNamed { Long64_t fSeekKey; ///< Location of object on file Long64_t fSeekPdir; ///< Location of parent directory on file TString fClassName; ///< Object Class name - Int_t fLeft; ///< Number of bytes left in current segment + Long64_t fLeft; ///< Number of bytes left in current segment char *fBuffer; ///< Object buffer TBuffer *fBufferRef; ///< Pointer to the TBuffer object UShort_t fPidOffset; /// #include +#include +#include #ifdef R__FBSD #include @@ -1504,27 +1506,58 @@ Bool_t TFile::IsOpen() const void TFile::MakeFree(Long64_t first, Long64_t last) { - TFree *f1 = (TFree*)fFree->First(); - if (!f1) return; - TFree *newfree = f1->AddFree(fFree,first,last); - if(!newfree) return; - Long64_t nfirst = newfree->GetFirst(); - Long64_t nlast = newfree->GetLast(); - Long64_t nbytesl= nlast-nfirst+1; - if (nbytesl > 2000000000) nbytesl = 2000000000; - Int_t nbytes = -Int_t (nbytesl); - char buffer[sizeof(Int_t)]; - char *pbuffer = buffer; - tobuf(pbuffer, nbytes); - if (last == fEND-1) fEND = nfirst; - Seek(nfirst); - // We could not update the meta data for this block on the file. - // This is not fatal as this only means that we won't get it 'right' - // if we ever need to Recover the file before the block is actually - // (attempted to be reused. - // coverity[unchecked_value] - WriteBuffer(buffer, sizeof(buffer)); - if (fMustFlush) Flush(); + assert(0 < first && first < last && last < fEND); + + TFree *f1 = static_cast(fFree->First()); + assert(f1); // There must always be at least the virtual free segment at the end of the file + + TFree *newfree = f1->AddFree(fFree, first, last); + assert(newfree); // AddFree() always succeeds + + const Long64_t nfirst = newfree->GetFirst(); + const Long64_t nlast = newfree->GetLast(); + assert(nfirst > 0 && nfirst <= first && nlast >= last); + Long64_t nbytesl = std::min(nlast, fEND) - nfirst + 1; + assert(nbytesl >= static_cast(sizeof(Int_t))); + + auto fnWriteGapHeader = [this](ULong64_t offset, ULong64_t gapSize) { + assert((gapSize <= TFile::kMaxGapSize) && (fEND > 0) && + ((offset + sizeof(Int_t)) <= static_cast(fEND))); + + auto nbytes = -static_cast(gapSize); + char buffer[sizeof(Int_t)]; + char *pbuffer = buffer; + tobuf(pbuffer, nbytes); + + Seek(offset); + if (WriteBuffer(buffer, sizeof(buffer)) != 0) { + // Not fatal, this only means that we won't get it 'right' + // if we ever need to Recover the file before the block is actually + // attempted to be reused. + Warning("TFile::MakeFree()", "failed to write free segment header"); + } + }; + + Long64_t offset = nfirst; + while (nbytesl > TFile::kMaxGapSize) { + // For gaps larger than 2GB, link several consecutive gaps together. This has to be done because the size + // marker on disk is 32 bits. The free list, however, will still have one large gap because the free list + // uses 64 bit [first..last] pairs to represent gaps. File recovery will merge consecutive gaps. + + // Make sure that the second gap is large enough to write its size on disk + Long64_t gapSize = TFile::kMaxGapSize - sizeof(Int_t); + fnWriteGapHeader(offset, gapSize); + + nbytesl -= gapSize; + offset += gapSize; + } + fnWriteGapHeader(offset, nbytesl); + + if (last == fEND - 1) + fEND = nfirst; + + if (fMustFlush) + Flush(); } //////////////////////////////////////////////////////////////////////////////// @@ -2125,7 +2158,14 @@ Int_t TFile::Recover() fEND = Long64_t(size); - if (fWritable && !fFree) fFree = new TList; + if (fWritable) { + if (fFree) { + // Remove an existing free list because we will recover it from the chain of segments + fFree->Delete(); + delete fFree; + } + fFree = new TList(); + } Int_t nrecov = 0; nwheader = 1024; @@ -2148,9 +2188,19 @@ Int_t TFile::Recover() break; } if (nbytes < 0) { + if ((-nbytes < static_cast(sizeof(Int_t))) || (-nbytes > static_cast(TFile::kMaxGapSize))) { + Error("Recover", "Address = %lld\tNbytes = %d\t=====E R R O R=======", idcur, nbytes); + break; + } + if (fWritable) { + const Long64_t last = idcur - nbytes - 1; + if (fFree->Last() && static_cast(fFree->Last())->GetLast() + 1 == idcur) { + static_cast(fFree->Last())->SetLast(last); + } else { + new TFree(fFree, idcur, last); + } + } idcur -= nbytes; - if (fWritable) new TFree(fFree,idcur,idcur-nbytes-1); - Seek(idcur); continue; } Version_t versionkey; @@ -2196,15 +2246,18 @@ Int_t TFile::Recover() idcur += nbytes; } if (fWritable) { - Long64_t max_file_size = Long64_t(kStartBigFile); - if (max_file_size < fEND) max_file_size = fEND+1000000000; - TFree *last = (TFree*)fFree->Last(); - if (last) { - last->AddFree(fFree,fEND,max_file_size); - } else { - new TFree(fFree,fEND,max_file_size); + if (fFree->Last() && static_cast(fFree->Last())->GetLast() == idcur - 1) { + // If the last recovered segment is a free segment, remove it and replace it by a newly created artificial one + fEND = static_cast(fFree->Last())->GetFirst(); + delete fFree->Last(); + fFree->Remove(fFree->LastLink()); } - if (nrecov) Write(); + Long64_t max_file_size = Long64_t(kStartBigFile); + if (max_file_size < fEND) + max_file_size = fEND + 1000000000; + new TFree(fFree, fEND, max_file_size); + if (nrecov) + Write(); } return nrecov; } diff --git a/io/io/src/TFree.cxx b/io/io/src/TFree.cxx index 5a06ac2497ada..fcdc3d06d9650 100644 --- a/io/io/src/TFree.cxx +++ b/io/io/src/TFree.cxx @@ -92,7 +92,10 @@ TFree *TFree::AddFree(TList *lfree, Long64_t first, Long64_t last) } idcur = (TFree*)lfree->After(idcur); } - return 0; + + // never here + assert(false); + return nullptr; } //////////////////////////////////////////////////////////////////////////////// @@ -126,7 +129,6 @@ void TFree::FillBuffer(char *&buffer) TFree *TFree::GetBestFree(TList *lfree, Int_t nbytes) { TFree *idcur = this; - if (idcur == 0) return 0; TFree *idcur1 = 0; do { Long64_t nleft = Long64_t(idcur->fLast - idcur->fFirst +1); @@ -186,4 +188,3 @@ Int_t TFree::Sizeof() const if (fLast > TFile::kStartBigFile) return 18; else return 10; } - diff --git a/io/io/src/TKey.cxx b/io/io/src/TKey.cxx index d74b2cc176d08..3e20e7d2754a0 100644 --- a/io/io/src/TKey.cxx +++ b/io/io/src/TKey.cxx @@ -540,7 +540,7 @@ void TKey::Create(Int_t nbytes, TFile* externFile) fLeft = -1; if (!fBuffer) fBuffer = new char[nsize]; } else { - fLeft = Int_t(bestfree->GetLast() - fSeekKey - nsize + 1); + fLeft = bestfree->GetLast() - fSeekKey - nsize + 1; } //*-*----------------- Case where new object fills exactly a deleted gap fNbytes = nsize; @@ -554,11 +554,11 @@ void TKey::Create(Int_t nbytes, TFile* externFile) //*-*----------------- Case where new object is placed in a deleted gap larger than itself if (fLeft > 0) { // found a bigger segment if (!fBuffer) { + // We reserve space for the new free segment size marker but we don't fill the buffer yet. + // We have to check (on writing, i.e. when we do I/O) if we are writing into a large gap (>2GB), + // in which case we need to read the first link size first. fBuffer = new char[nsize+sizeof(Int_t)]; } - char *buffer = fBuffer+nsize; - Int_t nbytesleft = -fLeft; // set header of remaining record - tobuf(buffer, nbytesleft); bestfree->SetFirst(fSeekKey+nsize); } @@ -1480,6 +1480,74 @@ void TKey::Streamer(TBuffer &b) } } +/// Fills zero, one, or two (rare) free segment header buffers into the provided `buffers` parameter. +/// If the key fits exactly in a provided gap or is at the end of the file, no buffer is filled. Otherwise, +/// the buffer for the integer immediately after the key data is filled with the size of the remaining, shortened gap. +/// In rare cases, it can be necessary to fill the second buffer with another free segment link. +/// Returns the number of buffers filled. +UShort_t TKey::FillGapHeaderBuffers(TFile &f, GapHeaderBuf_t &buffers) const +{ + if (fLeft <= 0) + return 0; + + // We must fill at least one free segment header + + if (fLeft <= TFile::kMaxGapSize) { + char *pbuf = buffers[0].data(); + tobuf(pbuf, -static_cast(fLeft)); + return 1; + } + + // Large gap, we need to read the free segment headers that are going to be overwritten by the key to find the + // first free segment header beyond the key. In fact, we must find a free segment header far enough beyond the + // key so that we can inject another, new free segment header between the key end and the existing one (which means + // key end + sizeof(int)) + const auto newGapOffset = fSeekKey + fNbytes; + auto existingGapOffset = fSeekKey; + auto prevGapOffset = 0; + do { + char readBuf[sizeof(Int_t)]; + Int_t header = 0; + + f.Seek(existingGapOffset); + if (f.ReadBuffer(readBuf, sizeof(Int_t))) { + Error("FillGapHeaderBuffers", "cannot read free segment link size at %lld", existingGapOffset); + return 0; + } + + char *pbuf = readBuf; + frombuf(pbuf, &header); + const Int_t gapSize = -header; + if (gapSize < static_cast(sizeof(Int_t)) || gapSize > TFile::kMaxGapSize || + gapSize > (newGapOffset - existingGapOffset + fLeft)) { + Error("FillGapHeaderBuffers", "invalid free segment header size %d at %lld", gapSize, existingGapOffset); + return 0; + } + + prevGapOffset = existingGapOffset; + existingGapOffset += gapSize; + } while (existingGapOffset <= newGapOffset + static_cast(sizeof(Int_t))); + + if ((prevGapOffset < newGapOffset) || (existingGapOffset - newGapOffset) <= TFile::kMaxGapSize) { + // Normal case: writing the new free segment header won't overwrite an existing one beyond the key. + // Rare case: the new free segment header will overwrite (part of) and existing one beyond the key. + // We can then lengthen the gap beteween [prevGapOffset, newGapOffset] except if this has already + // the maximum length. + char *pbuf = buffers[0].data(); + tobuf(pbuf, -static_cast(existingGapOffset - newGapOffset)); + return 1; + } + + // Very rare case: we need two free segment headers after the key + char *pbuf = buffers[0].data(); + tobuf(pbuf, -static_cast(sizeof(Int_t))); + const auto newGapSize = existingGapOffset - newGapOffset - sizeof(Int_t); + assert(newGapSize <= TFile::kMaxGapSize); + pbuf = buffers[1].data(); + tobuf(pbuf, -static_cast(newGapSize)); + return 2; +} + //////////////////////////////////////////////////////////////////////////////// /// Write the encoded object supported by this key. /// The function returns the number of bytes committed to the file. @@ -1498,9 +1566,21 @@ Int_t TKey::WriteFile(Int_t cycle, TFile* f) buffer = fBuffer; } - if (fLeft > 0) nsize += sizeof(Int_t); + GapHeaderBuf_t gapHeaderBuffers; + auto nGaps = FillGapHeaderBuffers(*f, gapHeaderBuffers); + assert(nGaps <= 2); + if (nGaps > 0) { + std::memcpy(fBuffer + fNbytes, gapHeaderBuffers[0].data(), sizeof(Int_t)); + nsize += sizeof(Int_t); + } + f->Seek(fSeekKey); Bool_t result = f->WriteBuffer(buffer,nsize); + + if (nGaps == 2) { + f->WriteBuffer(gapHeaderBuffers[1].data(), sizeof(Int_t)); + nsize += sizeof(Int_t); + } //f->Flush(); Flushing takes too much time. // Let user flush the file when they want. if (gDebug) { @@ -1525,9 +1605,21 @@ Int_t TKey::WriteFileKeepBuffer(TFile *f) Int_t nsize = fNbytes; char *buffer = fBuffer; - if (fLeft > 0) nsize += sizeof(Int_t); + GapHeaderBuf_t gapHeaderBuffers; + auto nGaps = FillGapHeaderBuffers(*f, gapHeaderBuffers); + assert(nGaps <= 2); + if (nGaps > 0) { + std::memcpy(fBuffer + fNbytes, gapHeaderBuffers[0].data(), sizeof(Int_t)); + nsize += sizeof(Int_t); + } + f->Seek(fSeekKey); Bool_t result = f->WriteBuffer(buffer,nsize); + + if (nGaps == 2) { + f->WriteBuffer(gapHeaderBuffers[1].data(), sizeof(Int_t)); + nsize += sizeof(Int_t); + } //f->Flush(); Flushing takes too much time. // Let user flush the file when they want. if (gDebug) { diff --git a/io/io/test/TFileTests.cxx b/io/io/test/TFileTests.cxx index 5ef2ed684feee..2760d20d95d3c 100644 --- a/io/io/test/TFileTests.cxx +++ b/io/io/test/TFileTests.cxx @@ -2,20 +2,22 @@ #include #include #include +#include #include "gtest/gtest.h" #include -#include "TFile.h" -#include "TMemFile.h" #include "TDirectory.h" +#include "TEnv.h" +#include "TFile.h" +#include "TFree.h" #include "TKey.h" +#include "TMemFile.h" #include "TNamed.h" #include "TPluginManager.h" -#include "TROOT.h" // gROOT +#include "TROOT.h" #include "TSystem.h" -#include "TEnv.h" // gEnv TEST(TFile, WriteObjectTObject) { @@ -332,3 +334,336 @@ TEST(TFile, UUID) TMemFile f("uuidtest.root", "RECREATE"); EXPECT_EQ('4', f.GetUUID().AsString()[14]); } + +TEST(TFile, DeleteKey) +{ + ROOT::TestSupport::FileRaii fileGuard("tfile_test_delete_keys.root"); + + auto fnCountGaps = [](const std::string &fileName) -> std::uint64_t { + auto f = std::unique_ptr(TFile::Open(fileName.c_str())); + std::uint64_t nGaps = 0; + for (const auto &k : f->WalkTKeys()) { + if (k.fLen == TFile::kMaxGapSize) { + // this used to indicate a truncated free segment (corrupt segment list). Gaps could still exactly + // be 2GB in size but not for the files in this unit test. + throw std::runtime_error("truncated free segment"); + } + if (k.fType == ROOT::Detail::TKeyMapNode::kGap) + nGaps++; + } + return nGaps; + }; + + auto f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "RECREATE")); + f->SetCompressionSettings(0); + f->Write(); + f->Close(); + + // The empty file should have no gaps. Note that gaps are created temporarily when certain keys are overwritten. + EXPECT_EQ(0, fnCountGaps(fileGuard.GetPath())); + + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "UPDATE")); + std::vector v; + f->WriteObject(&v, "va0"); + f->WriteObject(&v, "va1"); + f->WriteObject(&v, "va2"); + f->Write(); + f->Close(); + // 2 gaps: new (larger) keys list and free list are written + EXPECT_EQ(2, fnCountGaps(fileGuard.GetPath())); + + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "UPDATE")); + f->Delete("va1;*"); // should create small gap that cannot be merged, trapped between v0 and v2 + f->Write(); + f->Close(); + + EXPECT_EQ(3, fnCountGaps(fileGuard.GetPath())); + + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "UPDATE")); + f->Delete("va2;*"); // gaps at the tail should merge + f->Write(); + f->Close(); + + EXPECT_EQ(2, fnCountGaps(fileGuard.GetPath())); + + // The following tests run out of memory on 32bit platforms + if (sizeof(std::size_t) == 4) { + printf("Skipping test partially on 32bit platform.\n"); + return; + } + + v.resize(1000 * 1000, 'x'); // next few objects are 1MB in size + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "RECREATE")); + f->SetCompressionSettings(0); + f->WriteObject(&v, "vb0"); + f->WriteObject(&v, "vb1"); + f->WriteObject(&v, "vb2"); + f->WriteObject(&v, "vb3"); + v.resize(1000 * 1000 * 1000 - 100, 'x'); // almost 1GB + f->WriteObject(&v, "vc0"); + f->WriteObject(&v, "vc1"); + f->WriteObject(&v, "vc2"); + f->Write(); + EXPECT_GT(f->GetEND(), TFile::kStartBigFile); + f->Close(); + + // New file, no gaps + EXPECT_EQ(0, fnCountGaps(fileGuard.GetPath())); + + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "UPDATE")); + f->Delete("vb1;*"); // Make a medium sized gap into which smaller objects fit, e.g. the free list + f->Delete("vb3;*"); // | + f->Delete("vc0;*"); // | + f->Delete("vc1;*"); // |---> Single merged gap in free list, multi-hop free segment on disk + f->Write(); + f->Close(); + + // Free list in gap created by vb1, one gap at the end because we have a smaller keys list. Two consecutive + // gaps for removed vb3, vc0, vc1. + EXPECT_EQ(4, fnCountGaps(fileGuard.GetPath())); + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "UPDATE")); + EXPECT_FALSE(f->TestBit(TFile::kRecovered)); + // Only 3 real gaps plus one virtual gap at the end of the file + EXPECT_EQ(4, f->GetNfree()); + // Force the next open to recover the file + f->GetListOfKeys()->Clear(); + f->Write(); + f->Close(); + + { + ROOT::TestSupport::CheckDiagsRAII diagsRaii; + diagsRaii.requiredDiag(kInfo, "TFile::Recover", "recovered key vector", false); + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "UPDATE")); + EXPECT_TRUE(f->TestBit(TFile::kRecovered)); + // We got one more free gap due to the replacement of the empty keys list. Otherwise, we still want to see + // that the large gap was merged from the smaller segments. + EXPECT_EQ(5, f->GetNfree()); + bool foundLargeGap = false; + for (const auto gap : ROOT::Detail::TRangeStaticCast(f->GetListOfFree())) { + if (gap->GetLast() - gap->GetFirst() >= TFile::kMaxGapSize) { + foundLargeGap = true; + break; + } + } + EXPECT_TRUE(foundLargeGap); + f->Write(); + f->Close(); + } + // Same as before the recovery + EXPECT_EQ(4, fnCountGaps(fileGuard.GetPath())); + + // Write in large gap (between 1GB and 2GB), reproducer of issue https://github.com/root-project/root/issues/19245 + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "RECREATE")); + f->SetCompressionSettings(0); + + v.resize(1000 * 1000 * 1000 - 100, 'x'); // almost 1GB + f->WriteObject(&v, "big1"); + f->WriteObject(&v, "big2"); + v.resize(1024 * 1024, 'x'); + f->WriteObject(&v, "small1"); + f->Write(); + f->Close(); + + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "UPDATE")); + // Creates a combined gap close to 2GB + f->Delete("big1;*"); + f->Delete("big2;*"); + f->Write(); + f->Close(); + + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "UPDATE")); + f->WriteObject(&v, "small2"); + f->Write(); + f->Close(); + + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "UPDATE")); + EXPECT_FALSE(f->TestBit(TFile::kRecovered)); +} + +TEST(TFile, WriteInLargeGap) +{ + // The following tests run out of memory on 32bit platforms + if (sizeof(std::size_t) == 4) { + GTEST_SKIP() << "Skipping test on 32bit platform."; + } + + ROOT::TestSupport::FileRaii fileGuard("tfile_test_large_gap_at_end.root"); + auto f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "RECREATE")); + f->SetCompressionSettings(0); + std::vector v; + v.resize(1000 * 1000 * 1000 - 100, 'x'); // almost 1GB + f->WriteObject(&v, "v0"); + f->WriteObject(&v, "v1"); + f->WriteObject(&v, "v2"); + f->Write(); + f->Close(); + + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "UPDATE")); + EXPECT_GT(f->GetEND(), TFile::kStartBigFile); + f->Delete("v0;*"); + f->Delete("v1;*"); + f->Delete("v2;*"); + v.clear(); + f->WriteObject(&v, "small"); //< This should not crash, i.e. the new key should be placed in the large gap + + int nConsecutiveGapsAfterSmall = -1; + for (const auto &k : f->WalkTKeys()) { + if (k.fType == ROOT::Detail::TKeyMapNode::kGap && nConsecutiveGapsAfterSmall >= 0) { + nConsecutiveGapsAfterSmall++; + continue; + } + if (k.fKeyName == "small") { + nConsecutiveGapsAfterSmall = 0; + continue; + } + if (nConsecutiveGapsAfterSmall >= 0) + break; + } + EXPECT_EQ(2, nConsecutiveGapsAfterSmall); + + TNamed x("x", ""); + f->WriteObject(&x, "x"); //< Update the streamer info so that it doesn't block shrinking the file + f->Write(); + f->Close(); + + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str())); + EXPECT_LT(f->GetEND(), TFile::kStartBigFile); +} + +TEST(TFile, WriteInLargeGapCornerCase) +{ + // The following tests run out of memory on 32bit platforms + if (sizeof(std::size_t) == 4) { + GTEST_SKIP() << "Skipping test on 32bit platform."; + } + + // Constructs a case that requires writing 2 free segment headers after a key + + ROOT::TestSupport::FileRaii fileGuard("tfile_test_write_in_large_gap_corner_case.root"); + auto f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "RECREATE")); + f->SetCompressionSettings(0); + std::vector v; + v.resize(1000 * 1000 * 1000 - 100, 'x'); // almost 1GB + f->WriteObject(&v, "v0"); + f->WriteObject(&v, "v1"); + f->WriteObject(&v, "v2"); + f->Write(); + f->Delete("v0;*"); + f->Delete("v1;*"); + f->Delete("v2;*"); + f->Write(); + + // We should have two large consecutive gaps + Long64_t seekGap = 0; + Long64_t gapSize = 0; // the combined gap size + for (const auto &k : f->WalkTKeys()) { + if (seekGap > 0) { + // second gap + EXPECT_EQ(ROOT::Detail::TKeyMapNode::kGap, k.fType); + gapSize += k.fLen; + break; + } + if (k.fLen > 1000000) { + EXPECT_EQ(ROOT::Detail::TKeyMapNode::kGap, k.fType); + seekGap = k.fAddr; + gapSize = k.fLen; + } + } + ASSERT_GT(seekGap, 0); + ASSERT_GT(gapSize, TFile::kMaxGapSize); + + // Create new key in the large gap; not huge but large enough to only fit in the large gap + TKey testKey("TEST", "TITLE", TObject::Class(), 1024 * 1024, f.get()); + EXPECT_EQ(seekGap, testKey.GetSeekKey()); + + // Manipulate the linked list of free segments in the large gap: + // - 2 empty free gaps + // - 1 max sized gap 1 byte after the key end + // - final segment pointing to the original end + std::array buf; + char *pbuf = buf.data(); + + Long64_t offset = seekGap; + f->Seek(offset); + tobuf(pbuf, -static_cast(sizeof(Int_t))); + EXPECT_FALSE(f->WriteBuffer(buf.data(), sizeof(Int_t))); + + offset += sizeof(Int_t); + pbuf = buf.data(); + tobuf(pbuf, -static_cast(seekGap + testKey.GetNbytes() + 1 - offset)); + EXPECT_FALSE(f->WriteBuffer(buf.data(), sizeof(Int_t))); + + offset += testKey.GetNbytes() + 1 - sizeof(Int_t); + f->Seek(offset); + pbuf = buf.data(); + tobuf(pbuf, -static_cast(TFile::kMaxGapSize)); + EXPECT_FALSE(f->WriteBuffer(buf.data(), sizeof(Int_t))); + + offset += TFile::kMaxGapSize; + f->Seek(offset); + EXPECT_GT(seekGap + gapSize, offset); + EXPECT_LT(seekGap + gapSize - offset, TFile::kMaxGapSize); + pbuf = buf.data(); + tobuf(pbuf, -static_cast(seekGap + gapSize - offset)); + EXPECT_FALSE(f->WriteBuffer(buf.data(), sizeof(Int_t))); + + testKey.WriteFile(1, f.get()); + + int step = 0; + for (const auto &k : f->WalkTKeys()) { + if (step == 3) { + EXPECT_EQ(ROOT::Detail::TKeyMapNode::kGap, k.fType); + EXPECT_EQ(k.fAddr + k.fLen, seekGap + gapSize); + step = 4; + } else if (step == 2) { + EXPECT_EQ(ROOT::Detail::TKeyMapNode::kGap, k.fType); + EXPECT_LT(k.fLen, TFile::kMaxGapSize); + step = 3; + } else if (step == 1) { + EXPECT_EQ(ROOT::Detail::TKeyMapNode::kGap, k.fType); + EXPECT_EQ(sizeof(Int_t), k.fLen); + step = 2; + } else if (k.fAddr == static_cast(seekGap)) { + EXPECT_EQ(ROOT::Detail::TKeyMapNode::kKey, k.fType); + EXPECT_EQ(k.fLen, testKey.GetNbytes()); + step = 1; + } + } + EXPECT_EQ(4, step); + + f->Write(); + f->Close(); +} + +TEST(TFile, KeySizeLimit) +{ + // The following tests run out of memory on 32bit platforms + if (sizeof(std::size_t) == 4) { + GTEST_SKIP() << "Skipping test on 32bit platform."; + } + + ROOT::TestSupport::FileRaii fileGuard("tfile_test_key_size_limit.root"); + + auto f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "RECREATE")); + f->SetCompressionSettings(0); + + // Check that we can add keys >1GB (but smaller than 1GiB, obviously) in small and large files. + // This does work even though the last, virtual free segment is 1GB (and not 1GiB). The reason it works is + // that when the last free segment is not large enough, the code path that supports upgrading from a small file + // to a large file is activated and extends the last free segment as needed. + + std::vector v; + v.resize(1000 * 1000 * 1000 + 100, 'x'); // more than 1GB but less the 1GiB + f->WriteObject(&v, "v0"); + EXPECT_LT(f->GetEND(), TFile::kStartBigFile); + f->WriteObject(&v, "v1"); + EXPECT_GT(f->GetEND(), TFile::kStartBigFile); + f->Write(); + f->Close(); + + f = std::unique_ptr(TFile::Open(fileGuard.GetPath().c_str(), "UPDATE")); + EXPECT_GT(f->GetEND(), TFile::kStartBigFile); + f->WriteObject(&v, "v2"); + f->Write(); + f->Close(); +} diff --git a/tree/ntuple/src/RMiniFile.cxx b/tree/ntuple/src/RMiniFile.cxx index 35f28cb666b71..9d07c7112ad22 100644 --- a/tree/ntuple/src/RMiniFile.cxx +++ b/tree/ntuple/src/RMiniFile.cxx @@ -618,6 +618,8 @@ struct RTFileControlBlock { /// on some platforms. class RKeyBlob : public TKey { public: + using TKey::GapHeaderBuf_t; + RKeyBlob() = default; explicit RKeyBlob(TFile *file) : TKey(file) @@ -634,7 +636,7 @@ class RKeyBlob : public TKey { *seekKey = fSeekKey; } - bool WasAllocatedInAFreeSlot() const { return fLeft > 0; } + using TKey::FillGapHeaderBuffers; ClassDefInlineOverride(RKeyBlob, 0) }; @@ -1173,11 +1175,10 @@ std::uint64_t ROOT::Internal::RNTupleFileWriter::ReserveBlobKey(T &caller, TFile caller.Write(localKeyBuffer, kBlobKeyLen, offsetKey); } - if (keyBlob.WasAllocatedInAFreeSlot()) { - // If the key was allocated in a free slot, the last 4 bytes of its buffer contain the new size - // of the remaining free slot and we need to write it to disk before the key gets destroyed at the end of the - // function. - caller.Write(keyBlob.GetBuffer() + nbytes, sizeof(Int_t), offsetKey + kBlobKeyLen + nbytes); + RKeyBlob::GapHeaderBuf_t buffers; + auto nGaps = keyBlob.FillGapHeaderBuffers(file, buffers); + for (unsigned int i = 0; i < nGaps; ++i) { + caller.Write(buffers[i].data(), sizeof(Int_t), offsetKey + kBlobKeyLen + nbytes + i * sizeof(Int_t)); } auto offsetData = offsetKey + kBlobKeyLen; diff --git a/tree/tree/test/CMakeLists.txt b/tree/tree/test/CMakeLists.txt index 4e903b06daafc..2bff62cc1e114 100644 --- a/tree/tree/test/CMakeLists.txt +++ b/tree/tree/test/CMakeLists.txt @@ -30,6 +30,7 @@ if(imt) endif() ROOT_ADD_GTEST(testTChainSaveAsCxx TChainSaveAsCxx.cxx LIBRARIES RIO Tree) ROOT_ADD_GTEST(testTChainRegressions TChainRegressions.cxx LIBRARIES RIO Tree) +ROOT_ADD_GTEST(testTTreeOverwrite TTreeOverwrite.cxx LIBRARIES RIO Tree) ROOT_ADD_GTEST(testTTreeTruncatedDatatypes TTreeTruncatedDatatypes.cxx LIBRARIES RIO Tree) ROOT_ADD_GTEST(testTTreeRegressions TTreeRegressions.cxx LIBRARIES RIO Tree Hist) ROOT_ADD_GTEST(entrylist_addsublist entrylist_addsublist.cxx LIBRARIES RIO Tree) @@ -44,4 +45,3 @@ ROOT_GENERATE_DICTIONARY(EvolutionStruct ${CMAKE_CURRENT_SOURCE_DIR}/EvolutionSt LINKDEF EvolutionStructLinkDef.h OPTIONS -inlineInputHeader DEPENDENCIES RIO) - diff --git a/tree/tree/test/TTreeOverwrite.cxx b/tree/tree/test/TTreeOverwrite.cxx new file mode 100644 index 0000000000000..e44d8cef90a3b --- /dev/null +++ b/tree/tree/test/TTreeOverwrite.cxx @@ -0,0 +1,40 @@ +#include "gtest/gtest.h" + +#include + +#include +#include + +TEST(TTree, Overwrite) +{ + ROOT::TestSupport::FileRaii fileGuard("test_ttree_overwrite.root"); + + auto f = TFile::Open(fileGuard.GetPath().c_str(), "RECREATE"); + auto t = new TTree("t", ""); + f->Write(); + f->Close(); + delete f; + + f = TFile::Open(fileGuard.GetPath().c_str(), "UPDATE"); + EXPECT_FALSE(f->TestBit(TFile::kRecovered)); + t = f->Get("t"); + t->Delete("all"); + f->Write(); + f->Close(); + delete f; + + f = TFile::Open(fileGuard.GetPath().c_str(), "UPDATE"); + // Empty files are always "recovered" because they don't contain keys; however, TFile::Recover() doesn't + // set the kRecovered bit because there are no keys to recover. + EXPECT_FALSE(f->TestBit(TFile::kRecovered)); + t = new TTree("t", ""); + f->Write(); + f->Close(); + delete f; + + f = TFile::Open(fileGuard.GetPath().c_str(), "UPDATE"); + EXPECT_FALSE(f->TestBit(TFile::kRecovered)); + EXPECT_NE(nullptr, f->Get("t")); + f->Close(); + delete f; +}