Skip to content

Commit

Permalink
[io] do not use single-file-merge shortcut if filters are specified
Browse files Browse the repository at this point in the history
Fixes #13359
  • Loading branch information
ferdymercury authored Apr 5, 2024
1 parent 0f5eb94 commit e32e317
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 12 deletions.
23 changes: 13 additions & 10 deletions io/io/src/TFileMerger.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,6 @@ Bool_t TFileMerger::MergeOne(TDirectory *target, TList *sourcelist, Int_t type,
}
// Check if only the listed objects are to be merged
if (type & kOnlyListed) {
onlyListed = kFALSE;
oldkeyname = keyname;
oldkeyname += " ";
onlyListed = fObjectNames.Contains(oldkeyname);
Expand Down Expand Up @@ -909,14 +908,18 @@ Bool_t TFileMerger::MergeRecursive(TDirectory *target, TList *sourcelist, Int_t
/// the file "FileMerger.root" in the working directory. Returns true
/// on success, false in case of error.
/// The type is defined by the bit values in EPartialMergeType:
/// kRegular : normal merge, overwritting the output file
/// kIncremental : merge the input file with the content of the output file (if already exising) (default)
/// kAll : merge all type of objects (default)
/// kResetable : merge only the objects with a MergeAfterReset member function.
/// kNonResetable : merge only the objects without a MergeAfterReset member function.
/// kRegular : normal merge, overwritting the output file
/// kIncremental : merge the input file with the content of the output file (if already exising) (default)
/// kResetable : merge only the objects with a MergeAfterReset member function.
/// kNonResetable : merge only the objects without a MergeAfterReset member function.
/// kDelayWrite : delay the TFile write (to reduce the number of write when reusing the file)
/// kAll : merge all type of objects (default)
/// kAllIncremental : merge incrementally all type of objects.
/// kOnlyListed : merge only the objects specified in fObjectNames list
/// kSkipListed : skip objects specified in fObjectNames list
/// kKeepCompression: keep compression level unchanged for each input
///
/// If the type is set to kIncremental the output file is done deleted at the end of
/// this operation. If the type is not set to kIncremental, the output file is closed.
/// If the type is not set to kIncremental, the output file is deleted at the end of this operation.

Bool_t TFileMerger::PartialMerge(Int_t in_type)
{
Expand All @@ -933,9 +936,9 @@ Bool_t TFileMerger::PartialMerge(Int_t in_type)
}
}

// Special treament for the single file case ...
// Special treament for the single file case to improve efficiency...
if ((fFileList.GetEntries() == 1) && !fExcessFiles.GetEntries() &&
!(in_type & kIncremental) && !fCompressionChange && !fExplicitCompLevel) {
!(in_type & (kIncremental | kOnlyListed | kSkipListed | kResetable | kNonResetable)) && !fCompressionChange && !fExplicitCompLevel) {
fOutputFile->Close();
SafeDelete(fOutputFile);

Expand Down
2 changes: 1 addition & 1 deletion io/io/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ ROOT_ADD_GTEST(TFile TFileTests.cxx LIBRARIES RIO)
ROOT_ADD_GTEST(TBufferFile TBufferFileTests.cxx LIBRARIES RIO)
ROOT_ADD_GTEST(TBufferMerger TBufferMerger.cxx LIBRARIES RIO Imt Tree)
ROOT_ADD_GTEST(TBufferJSON TBufferJSONTests.cxx LIBRARIES RIO)
ROOT_ADD_GTEST(TFileMerger TFileMergerTests.cxx LIBRARIES RIO Tree)
ROOT_ADD_GTEST(TFileMerger TFileMergerTests.cxx LIBRARIES RIO Tree Hist)
ROOT_ADD_GTEST(TROMemFile TROMemFileTests.cxx LIBRARIES RIO Tree)
if(uring AND NOT DEFINED ENV{ROOTTEST_IGNORE_URING})
ROOT_ADD_GTEST(RIoUring RIoUring.cxx LIBRARIES RIO)
Expand Down
35 changes: 34 additions & 1 deletion io/io/test/TFileMergerTests.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "TMemFile.h"
#include "TTree.h"
#include "TH1.h"

static void CreateATuple(TMemFile &file, const char *name, double value)
{
Expand Down Expand Up @@ -51,7 +52,8 @@ TEST(TFileMerger, CreateWithTFilePointer)
merger.AddFile(&a, false);
merger.AddFile(&b, false);
// FIXME: Calling merger.Merge() will call Close() and *delete* output.
merger.PartialMerge();
success = merger.PartialMerge();
ASSERT_TRUE(success);

auto &result = *static_cast<TMemFile *>(merger.GetOutputFile());
CheckTree(result, "a_tree", 1);
Expand All @@ -68,3 +70,34 @@ TEST(TFileMerger, CreateWithUnwritableTFilePointer)
ROOT_EXPECT_ERROR(merger.OutputFile(std::move(output)), "TFileMerger::OutputFile",
"output file output.root is not writable");
}

TEST(TFileMerger, MergeSingleOnlyListed)
{
TMemFile a("hist4.root", "CREATE");

auto hist1 = new TH1F("hist1", "hist1", 1 , 0 , 2);
auto hist2 = new TH1F("hist2", "hist2", 1 , 0 , 2);
auto hist3 = new TH1F("hist3", "hist3", 1 , 0 , 2);
auto hist4 = new TH1F("hist4", "hist4", 1 , 0 , 2);
hist1->Fill(1);
hist2->Fill(1); hist2->Fill(2);
hist3->Fill(1); hist3->Fill(1); hist3->Fill(1);
hist4->Fill(1); hist4->Fill(1); hist4->Fill(1); hist4->Fill(1);
a.Write();

TFileMerger merger;
auto output = std::unique_ptr<TFile>(new TFile("SingleOnlyListed.root", "RECREATE"));
bool success = merger.OutputFile(std::move(output));
ASSERT_TRUE(success);

merger.AddObjectNames("hist1");
merger.AddObjectNames("hist2");
merger.AddFile(&a, false);
const Int_t mode = (TFileMerger::kAll | TFileMerger::kRegular | TFileMerger::kOnlyListed);
success = merger.PartialMerge(mode); // This will delete fOutputFile as we are not using kIncremental, so merger.GetOutputFile() will return a nullptr
ASSERT_TRUE(success);

output = std::unique_ptr<TFile>(TFile::Open("SingleOnlyListed.root"));
ASSERT_TRUE(output.get() && output->GetListOfKeys());
EXPECT_EQ(output->GetListOfKeys()->GetSize(), 2);
}

0 comments on commit e32e317

Please sign in to comment.