Skip to content

[ntuple] Support strings longer than 254 chars in RMiniFile #24152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kr1xh wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[ntuple] Support strings longer than 254 chars in RMiniFile #24152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 77 additions & 58 deletions tree/ntuple/src/RMiniFile.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,29 +155,54 @@ class RUInt64BE {
}
};

#pragma pack(push, 1)
/// A name (type, identifier, ...) in the TFile binary format
struct RTFString {
unsigned char fLName{0};
char fData[255];
RTFString() = default;
RTFString(const std::string &str)
{
// The length of strings with 255 characters and longer are encoded with a 32-bit integer following the first
// byte. This is currently not handled.
R__ASSERT(str.length() < 255);
fLName = static_cast<unsigned char>(str.length());
memcpy(fData, str.data(), fLName);
}
std::size_t GetSize() const
/// A name (type, identifier, ...) in the TFile binary format, serialized as by TBufferFile::WriteTString():
/// strings with up to 254 characters are prefixed by a one byte length. Longer strings are prefixed by the byte 255
/// followed by the length as a 32-bit big-endian integer.
class RTFString {
std::string fSerialized;

public:
static constexpr unsigned char kLongStringMarker = 255;

RTFString() : fSerialized(1, '\0') {}
RTFString(std::string_view str)
{
// A length of 255 is special and means that the first byte is followed by a 32-bit integer with the actual
// length.
R__ASSERT(fLName != 255);
return 1 + fLName;
if (str.length() < kLongStringMarker) {
fSerialized.push_back(static_cast<char>(str.length()));
} else {
if (str.length() > static_cast<std::size_t>(std::numeric_limits<std::int32_t>::max()))
throw ROOT::RException(R__FAIL("string too long for the TFile format"));
RUInt32BE length{static_cast<std::uint32_t>(str.length())};
fSerialized.push_back(static_cast<char>(kLongStringMarker));
fSerialized.append(reinterpret_cast<const char *>(&length), sizeof(length));
}
fSerialized.append(str);
}
/// The size of the serialized string, including the length prefix
std::size_t GetSize() const { return fSerialized.size(); }
const char *GetData() const { return fSerialized.data(); }
};

/// Reads the TFile string at the given offset. Returns the string and its size on disk including the length prefix.
std::pair<std::string, std::uint64_t> ReadTFString(ROOT::Internal::RMiniFileReader &reader, std::uint64_t offset)
{
unsigned char shortLength = 0;
reader.ReadBuffer(&shortLength, 1, offset);
std::uint64_t prefixSize = 1;
std::uint64_t length = shortLength;
if (shortLength == RTFString::kLongStringMarker) {
RUInt32BE longLength;
reader.ReadBuffer(&longLength, sizeof(longLength), offset + prefixSize);
prefixSize += sizeof(longLength);
length = longLength;
}
std::string str(length, '\0');
if (length > 0)
reader.ReadBuffer(str.data(), length, offset + prefixSize);
return {std::move(str), prefixSize + length};
}

#pragma pack(push, 1)
/// The timestamp format used in TFile; the default constructor initializes with the current time
struct RTFDatetime {
RUInt32BE fDatetime;
Expand Down Expand Up @@ -222,7 +247,11 @@ struct RTFKey {
// For writing, we always produce "big" keys with 64-bit SeekKey and SeekPdir.
fVersion = fVersion + kBigKeyVersion;
fObjLen = szObjInMem;
fKeyLen = static_cast<RUInt16BE>(GetHeaderSize() + clName.GetSize() + objName.GetSize() + titleName.GetSize());
const auto keyLen = GetHeaderSize() + clName.GetSize() + objName.GetSize() + titleName.GetSize();
// TKey stores the key length as a signed 16-bit integer
if (keyLen > static_cast<std::size_t>(std::numeric_limits<std::int16_t>::max()))
throw ROOT::RException(R__FAIL("TFile key header too long: " + std::to_string(keyLen) + " bytes"));
fKeyLen = static_cast<std::uint16_t>(keyLen);
fInfoLong.fSeekKey = seekKey;
fInfoLong.fSeekPdir = seekPdir;
// Depends on fKeyLen being set
Expand Down Expand Up @@ -697,17 +726,13 @@ std::uint64_t ROOT::Internal::RMiniFileReader::SearchInDirectory(std::uint64_t &
auto offsetNextKey = offset + key.fKeyLen;

offset += key.GetHeaderSize();
RTFString name;
ReadBuffer(&name, 1, offset);
ReadBuffer(&name, name.GetSize(), offset);
if (std::string_view(name.fData, name.fLName) != typeName) {
const auto [className, classNameSize] = ReadTFString(*this, offset);
if (className != typeName) {
offset = offsetNextKey;
continue;
}
offset += name.GetSize();
ReadBuffer(&name, 1, offset);
ReadBuffer(&name, name.GetSize(), offset);
if (std::string_view(name.fData, name.fLName) == keyName) {
offset += classNameSize;
if (ReadTFString(*this, offset).first == keyName) {
return key.GetSeekKey();
}
offset = offsetNextKey;
Expand Down Expand Up @@ -769,15 +794,12 @@ ROOT::RResult<ROOT::RNTuple> ROOT::Internal::RMiniFileReader::GetNTupleProper(st
fNbytesKeyAndInfo = fileHeader.GetNbytesInfo();

RTFKey key;
RTFString name;
ReadBuffer(&key, sizeof(key), fileHeader.fBEGIN);
// Skip over the entire key length, including the class name, object name, and title stored in it.
std::uint64_t offset = fileHeader.fBEGIN + key.fKeyLen;
// Skip over the name and title of the TNamed preceding the TFile (root TDirectory) entry.
ReadBuffer(&name, 1, offset);
offset += name.GetSize();
ReadBuffer(&name, 1, offset);
offset += name.GetSize();
offset += ReadTFString(*this, offset).second;
offset += ReadTFString(*this, offset).second;

// split ntupleName by '/' character to open datasets in subdirectories.
std::string ntuplePathTail(ntuplePath);
Expand Down Expand Up @@ -866,16 +888,13 @@ ROOT::RResult<ROOT::RNTuple> ROOT::Internal::RMiniFileReader::GetNTupleBare(std:
{
RBareFileHeader fileHeader;
ReadBuffer(&fileHeader, sizeof(fileHeader), 0);
RTFString name;
auto offset = sizeof(fileHeader);
ReadBuffer(&name, 1, offset);
ReadBuffer(&name, name.GetSize(), offset);
std::string_view foundName(name.fData, name.fLName);
const auto [foundName, foundNameSize] = ReadTFString(*this, offset);
if (foundName != ntupleName) {
return R__FAIL("expected RNTuple named '" + std::string(ntupleName) + "' but instead found '" +
std::string(foundName) + "' in file '" + fRawFile->GetUrl() + "'");
return R__FAIL("expected RNTuple named '" + std::string(ntupleName) + "' but instead found '" + foundName +
"' in file '" + fRawFile->GetUrl() + "'");
}
offset += name.GetSize();
offset += foundNameSize;

RTFNTuple ntuple;
ReadBuffer(&ntuple, sizeof(ntuple), offset);
Expand Down Expand Up @@ -972,11 +991,11 @@ void ROOT::Internal::RNTupleFileWriter::PrepareBlobKey(std::int64_t offset, size
unsigned char *writeBuffer = buffer;
memcpy(writeBuffer, &keyHeader, keyHeader.GetHeaderSize());
writeBuffer += keyHeader.GetHeaderSize();
memcpy(writeBuffer, &strClass, strClass.GetSize());
memcpy(writeBuffer, strClass.GetData(), strClass.GetSize());
writeBuffer += strClass.GetSize();
memcpy(writeBuffer, &strObject, strObject.GetSize());
memcpy(writeBuffer, strObject.GetData(), strObject.GetSize());
writeBuffer += strObject.GetSize();
memcpy(writeBuffer, &strTitle, strTitle.GetSize());
memcpy(writeBuffer, strTitle.GetData(), strTitle.GetSize());
writeBuffer += strTitle.GetSize();
R__ASSERT(writeBuffer == buffer + kBlobKeyLen);
}
Expand Down Expand Up @@ -1142,9 +1161,9 @@ ROOT::Internal::RNTupleFileWriter::RImplSimple::WriteKey(const void *buffer, std

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());
Write(strClass.GetData(), strClass.GetSize());
Write(strObject.GetData(), strObject.GetSize());
Write(strTitle.GetData(), strTitle.GetSize());
auto offsetData = shared.fFilePos;
// The next key starts after the data.
shared.fKeyOffset = offsetData + nbytes;
Expand Down Expand Up @@ -1577,7 +1596,7 @@ void ROOT::Internal::RNTupleFileWriter::WriteBareFileSkeleton(int defaultCompres
auto &fileSimple = std::get<RImplSimple>(fFile);
fileSimple.Write(&bareHeader, sizeof(bareHeader), 0);
RTFString ntupleName{fNTupleName};
fileSimple.Write(&ntupleName, ntupleName.GetSize());
fileSimple.Write(ntupleName.GetData(), ntupleName.GetSize());

// Write zero-initialized ntuple to reserve the space; will be overwritten on commit
RTFNTuple ntupleOnDisk;
Expand Down Expand Up @@ -1650,15 +1669,15 @@ void ROOT::Internal::RNTupleFileWriter::WriteTFileKeysList(std::uint64_t anchorS
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());
fileSimple.Write(strEmpty.GetData(), strEmpty.GetSize());
fileSimple.Write(strFileName.GetData(), strFileName.GetSize());
fileSimple.Write(strEmpty.GetData(), strEmpty.GetSize());
fileSimple.Write(&keyList, keyList.GetSize());
fileSimple.Write(&keyRNTuple, keyRNTuple.GetHeaderSize());
// Write class name, object name, and title for this key.
fileSimple.Write(&strRNTupleClass, strRNTupleClass.GetSize());
fileSimple.Write(&strRNTupleName, strRNTupleName.GetSize());
fileSimple.Write(&strEmpty, strEmpty.GetSize());
fileSimple.Write(strRNTupleClass.GetData(), strRNTupleClass.GetSize());
fileSimple.Write(strRNTupleName.GetData(), strRNTupleName.GetSize());
fileSimple.Write(strEmpty.GetData(), strEmpty.GetSize());
fileSimple.fShared->fControlBlock->fFileRecord.fNBytesKeys =
fileShared.fFilePos - fileSimple.fShared->fControlBlock->fFileRecord.GetSeekKeys();
fileShared.fKeyOffset = fileShared.fFilePos;
Expand Down Expand Up @@ -1736,12 +1755,12 @@ void ROOT::Internal::RNTupleFileWriter::WriteTFileSkeleton(int defaultCompressio

fileSimple.Write(&keyRoot, keyRoot.GetHeaderSize(), RTFHeader::kBEGIN);
// Write class name, object name, and title for the TFile key.
fileSimple.Write(&strTFile, strTFile.GetSize());
fileSimple.Write(&strFileName, strFileName.GetSize());
fileSimple.Write(&strEmpty, strEmpty.GetSize());
fileSimple.Write(strTFile.GetData(), strTFile.GetSize());
fileSimple.Write(strFileName.GetData(), strFileName.GetSize());
fileSimple.Write(strEmpty.GetData(), strEmpty.GetSize());
// Write the name and title of the TNamed preceding the TFile entry.
fileSimple.Write(&strFileName, strFileName.GetSize());
fileSimple.Write(&strEmpty, strEmpty.GetSize());
fileSimple.Write(strFileName.GetData(), strFileName.GetSize());
fileSimple.Write(strEmpty.GetData(), strEmpty.GetSize());
// Will be overwritten on commit
fileSimple.fShared->fControlBlock->fSeekFileRecord = fileSimple.fShared->fFilePos;
fileSimple.Write(&fileSimple.fShared->fControlBlock->fFileRecord,
Expand Down
126 changes: 123 additions & 3 deletions tree/ntuple/test/ntuple_minifile.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,9 +438,9 @@ TEST(MiniFile, LongString)
FileRaii fileGuard("test_ntuple_minifile_long_string.root");

static constexpr const char *LongString =
"This is a very long text with exactly 254 characters, which is the maximum that the RNTupleWriter can currently "
"store in a TFile header. For longer strings, a length of 255 is special and means that the first length byte is "
"followed by an integer length.";
"This is a very long text with exactly 254 characters, which is the longest string that a TFile key stores with "
"a one byte length prefix. For longer strings, a length of 255 is special and means that the first length byte "
"is followed by an integer length.";
std::unique_ptr<TFile> file(TFile::Open(fileGuard.GetPath().c_str(), "RECREATE", LongString));
auto writer = RNTupleFileWriter::Append("ntuple", *file, RNTupleWriteOptions::kDefaultMaxKeySize,
/*isAttributesRNTuple=*/false);
Expand All @@ -458,6 +458,126 @@ TEST(MiniFile, LongString)
EXPECT_EQ(offFooter, ntuple1.GetSeekFooter());
}

TEST(MiniFile, LongStringTitle)
{
// Strings longer than 254 characters are stored as a 255 marker byte followed by a 32-bit length
for (std::size_t len : {255u, 256u, 2000u}) {
SCOPED_TRACE("title length " + std::to_string(len));
FileRaii fileGuard("test_ntuple_minifile_long_string_title_" + std::to_string(len) + ".root");
const std::string title(len, 't');

std::uint64_t offHeader;
std::uint64_t offFooter;
{
std::unique_ptr<TFile> file(TFile::Open(fileGuard.GetPath().c_str(), "RECREATE", title.c_str()));
auto writer = RNTupleFileWriter::Append("ntuple", *file, RNTupleWriteOptions::kDefaultMaxKeySize,
/*isAttributesRNTuple=*/false);
char header = 'h';
char footer = 'f';
offHeader = writer->WriteNTupleHeader(&header, 1, 1);
offFooter = writer->WriteNTupleFooter(&footer, 1, 1);
writer->Commit();
}

auto rawFile = RRawFile::Create(fileGuard.GetPath());
RMiniFileReader reader(rawFile.get());
auto ntuple = reader.GetNTuple("ntuple").Inspect();
EXPECT_EQ(offHeader, ntuple.GetSeekHeader());
EXPECT_EQ(offFooter, ntuple.GetSeekFooter());
}
}

TEST(MiniFile, LongNTupleNameRaw)
{
FileRaii fileGuard("test_ntuple_minifile_long_ntuple_name_raw.ntuple");
const std::string ntupleName(300, 'n');

RNTupleWriteOptions options;
auto writer = RNTupleFileWriter::Recreate(ntupleName, fileGuard.GetPath(), EContainerFormat::kBare, options);
char header = 'h';
char footer = 'f';
auto offHeader = writer->WriteNTupleHeader(&header, 1, 1);
auto offFooter = writer->WriteNTupleFooter(&footer, 1, 1);
writer->Commit();

auto rawFile = RRawFile::Create(fileGuard.GetPath());
RMiniFileReader reader(rawFile.get());
auto ntuple = reader.GetNTuple(ntupleName).Inspect();
EXPECT_EQ(offHeader, ntuple.GetSeekHeader());
EXPECT_EQ(offFooter, ntuple.GetSeekFooter());
}

TEST(MiniFile, LongNTupleNameStream)
{
FileRaii fileGuard("test_ntuple_minifile_long_ntuple_name_stream.root");
const std::string ntupleName(300, 'n');

RNTupleWriteOptions options;
auto writer = RNTupleFileWriter::Recreate(ntupleName, fileGuard.GetPath(), EContainerFormat::kTFile, options);
char header = 'h';
char footer = 'f';
auto offHeader = writer->WriteNTupleHeader(&header, 1, 1);
auto offFooter = writer->WriteNTupleFooter(&footer, 1, 1);
writer->Commit();

auto rawFile = RRawFile::Create(fileGuard.GetPath());
RMiniFileReader reader(rawFile.get());
auto ntuple = reader.GetNTuple(ntupleName).Inspect();
EXPECT_EQ(offHeader, ntuple.GetSeekHeader());
EXPECT_EQ(offFooter, ntuple.GetSeekFooter());

auto file = std::unique_ptr<TFile>(TFile::Open(fileGuard.GetPath().c_str(), "READ"));
ASSERT_TRUE(file);
auto anchor = std::unique_ptr<ROOT::RNTuple>(file->Get<ROOT::RNTuple>(ntupleName.c_str()));
ASSERT_TRUE(anchor);
EXPECT_TRUE(IsEqual(ntuple, RNTupleTester(*anchor).GetAnchor()));
}

TEST(MiniFile, LongNTupleNameProper)
{
FileRaii fileGuard("test_ntuple_minifile_long_ntuple_name_proper.root");
const std::string ntupleName(300, 'n');

std::uint64_t offHeader;
std::uint64_t offFooter;
{
std::unique_ptr<TFile> file(TFile::Open(fileGuard.GetPath().c_str(), "RECREATE"));
auto writer = RNTupleFileWriter::Append(ntupleName, *file, RNTupleWriteOptions::kDefaultMaxKeySize,
/*isAttributesRNTuple=*/false);
char header = 'h';
char footer = 'f';
offHeader = writer->WriteNTupleHeader(&header, 1, 1);
offFooter = writer->WriteNTupleFooter(&footer, 1, 1);
writer->Commit();
}

auto rawFile = RRawFile::Create(fileGuard.GetPath());
RMiniFileReader reader(rawFile.get());
auto ntuple = reader.GetNTuple(ntupleName).Inspect();
EXPECT_EQ(offHeader, ntuple.GetSeekHeader());
EXPECT_EQ(offFooter, ntuple.GetSeekFooter());
}

TEST(MiniFile, KeyHeaderTooLong)
{
FileRaii fileGuard("test_ntuple_minifile_key_header_too_long.root");
// TKey stores the key length as a signed 16-bit integer, which limits the total length of the names in a key
const std::string ntupleName(40000, 'n');

RNTupleWriteOptions options;
auto writer = RNTupleFileWriter::Recreate(ntupleName, fileGuard.GetPath(), EContainerFormat::kTFile, options);
char header = 'h';
char footer = 'f';
writer->WriteNTupleHeader(&header, 1, 1);
writer->WriteNTupleFooter(&footer, 1, 1);
try {
writer->Commit();
FAIL() << "key headers longer than 32767 bytes should throw";
} catch (const ROOT::RException &err) {
EXPECT_THAT(err.what(), testing::HasSubstr("TFile key header too long"));
}
}

TEST(MiniFile, MultiKeyBlob)
{
FileRaii fileGuard("test_ntuple_minifile_multi_key_blob.root");
Expand Down