Skip to content

Commit 09010c5

Browse files
committed
[io] make buffering state explicit in RRawFile
1 parent 9e5db78 commit 09010c5

File tree

6 files changed

+16
-21
lines changed

6 files changed

+16
-21
lines changed

io/io/inc/ROOT/RRawFile.hxx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,13 @@ public:
4747

4848
/// On construction, an ROptions parameter can customize the RRawFile behavior
4949
struct ROptions {
50+
static constexpr size_t kUseDefaultBlockSize = std::size_t(-1); ///< Use protocol-dependent default block size
51+
5052
ELineBreaks fLineBreak = ELineBreaks::kAuto;
51-
/// Read at least fBlockSize bytes at a time. A value of zero turns off I/O buffering. A negative value indicates
52-
/// that the protocol-dependent default block size should be used.
53+
/// Read at least fBlockSize bytes at a time. A value of zero turns off I/O buffering.
5354
/// After construction, a negative block size is used to store the block size value when buffering is turned off
5455
/// (see `SetBuffering()`).
55-
int fBlockSize = -1;
56+
size_t fBlockSize = kUseDefaultBlockSize;
5657
// Define an empty constructor to work around a bug in Clang: https://github.com/llvm/llvm-project/issues/36032
5758
ROptions() {}
5859
};
@@ -119,6 +120,8 @@ private:
119120
std::uint64_t fFileSize = kUnknownFileSize;
120121
/// Files are opened lazily and only when required; the open state is kept by this flag
121122
bool fIsOpen = false;
123+
/// Runtime switch to decide if reads are buffered or directly sent to ReadAtImpl()
124+
bool fIsBuffering = true;
122125

123126
protected:
124127
std::string fUrl;
@@ -180,7 +183,7 @@ public:
180183
/// Turn off buffered reads; all scalar read requests go directly to the implementation. Buffering can be turned
181184
/// back on.
182185
void SetBuffering(bool value);
183-
bool IsBuffering() const { return fOptions.fBlockSize > 0; }
186+
bool IsBuffering() const { return fIsBuffering; }
184187

185188
/// Read the next line starting from the current value of fFilePos. Returns false if the end of the file is reached.
186189
bool Readln(std::string &line);

io/io/src/RRawFile.cxx

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,9 @@ size_t ROOT::Internal::RRawFile::ReadAt(void *buffer, size_t nbytes, std::uint64
151151
return 0;
152152

153153
// "Large" reads are served directly, bypassing the cache; since nbytes > 0, fBlockSize == 0 is also handled here
154-
if ((fOptions.fBlockSize < 0) || nbytes > static_cast<unsigned int>(fOptions.fBlockSize))
154+
if (!fIsBuffering || nbytes > static_cast<unsigned int>(fOptions.fBlockSize))
155155
return ReadAtImpl(buffer, nbytes, offset);
156156

157-
R__ASSERT(fOptions.fBlockSize > 0);
158-
159157
if (!fBufferSpace) {
160158
fBufferSpace.reset(new unsigned char[kNumBlockBuffers * fOptions.fBlockSize]);
161159
for (unsigned int i = 0; i < kNumBlockBuffers; ++i) {
@@ -201,16 +199,9 @@ void ROOT::Internal::RRawFile::ReadV(RIOVec *ioVec, unsigned int nReq)
201199

202200
void ROOT::Internal::RRawFile::SetBuffering(bool value)
203201
{
204-
if (value) {
205-
if (fOptions.fBlockSize < 0) {
206-
fOptions.fBlockSize = -fOptions.fBlockSize;
207-
}
208-
} else {
209-
if (fOptions.fBlockSize > 0) {
210-
fOptions.fBlockSize = -fOptions.fBlockSize;
211-
fBufferSpace.reset();
212-
}
213-
}
202+
fIsBuffering = value;
203+
if (!fIsBuffering)
204+
fBufferSpace.reset();
214205
}
215206

216207
bool ROOT::Internal::RRawFile::Readln(std::string &line)

io/io/src/RRawFileUnix.cxx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ void ROOT::Internal::RRawFileUnix::OpenImpl()
7272
throw std::runtime_error("Cannot open '" + fUrl + "', error: " + std::string(strerror(errno)));
7373
}
7474

75-
if (fOptions.fBlockSize >= 0)
75+
if (fOptions.fBlockSize != ROptions::kUseDefaultBlockSize)
7676
return;
7777

7878
#ifdef R__SEEK64

io/io/src/RRawFileWin.cxx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ void ROOT::Internal::RRawFileWin::OpenImpl()
5858
// Prevent double buffering
5959
int res = setvbuf(fFilePtr, nullptr, _IONBF, 0);
6060
R__ASSERT(res == 0);
61-
if (fOptions.fBlockSize < 0)
61+
if (fOptions.fBlockSize == ROptions::kUseDefaultBlockSize)
6262
fOptions.fBlockSize = kDefaultBlockSize;
6363
}
6464

net/davix/src/RRawFileDavix.cxx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ void ROOT::Internal::RRawFileDavix::OpenImpl()
7575
if (fFileDes->fd == nullptr) {
7676
throw std::runtime_error("Cannot open '" + fUrl + "', error: " + err->getErrMsg());
7777
}
78-
if (fOptions.fBlockSize < 0)
78+
if (fOptions.fBlockSize == ROptions::kUseDefaultBlockSize)
7979
fOptions.fBlockSize = kDefaultBlockSize;
8080
}
8181

net/netxng/src/RRawFileNetXNG.cxx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ void ROOT::Internal::RRawFileNetXNG::OpenImpl()
7777
if( !st.IsOK() )
7878
throw std::runtime_error( "Cannot open '" + fUrl + "', " +
7979
st.ToString() + "; " + st.GetErrorMessage() );
80-
if( fOptions.fBlockSize < 0 ) fOptions.fBlockSize = kDefaultBlockSize;
80+
if (fOptions.fBlockSize == ROptions::kUseDefaultBlockSize)
81+
fOptions.fBlockSize = kDefaultBlockSize;
8182
}
8283

8384
size_t ROOT::Internal::RRawFileNetXNG::ReadAtImpl(void *buffer, size_t nbytes, std::uint64_t offset)

0 commit comments

Comments
 (0)