Skip to content

Commit

Permalink
Switch on length_read != nullptr in inline functions rather than in…
Browse files Browse the repository at this point in the history 
… virtual

slow functions, so that most of the time the switch happens at compile time.

Reduce inlined code size by avoiding `RIEGELI_CHECK` in inlined functions of
`Reader`. A check is generally required for checking for size overflow.

* In the case of `std::string`, rely on the check done by
  `std::string::{assign,append}()`, or move it to non-inline non-virtual
  `Reader::ReadSlow(std::string&)` where a numeric type could overflow before
  `std::string` could perform a check.

* In the case of `Chain` (which needs a check in `ReadAndAppend()`), rely
  on the check done by `Chain::Append()`, and check in a separate non-inline
  non-virtual `ReadSlowWithSizeCheck()` before calling virtual `ReadSlow()`.

* In the case of `absl::Cord` (which needs a check in `ReadAndAppend()`), do not
  use the fast path if the `absl::Cord::Append()` could overflow (it does not
  check the size itself), and move the check to separate non-inline non-virtual
  `ReadSlowWithSizeCheck()` before calling virtual `ReadSlow()`.

Make `Reader::ReadSlow(Cord&)` with `length_read` non-virtual. It was virtual by
mistake.

PiperOrigin-RevId: 604329798
  • Loading branch information
@QrczakMK
QrczakMK committed Feb 5, 2024
1 parent 20068a7 commit 10baed7
Show file tree
Hide file tree
Showing 2 changed files with 156 additions and 87 deletions.
123 changes: 71 additions & 52 deletions riegeli/bytes/reader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,68 +89,73 @@ bool Reader::ReadSlow(size_t length, char* dest) {
return true;
}

bool Reader::ReadSlow(size_t length, char* dest, size_t* length_read) {
bool Reader::ReadSlow(size_t length, char* dest, size_t& length_read) {
RIEGELI_ASSERT_LT(available(), length)
<< "Failed precondition of Reader::ReadSlow(char*): "
"enough data available, use Read(char*) instead";
if (length_read == nullptr) return ReadSlow(length, dest);
const Position pos_before = pos();
const bool read_ok = ReadSlow(length, dest);
RIEGELI_ASSERT_GE(pos(), pos_before)
<< "Reader::ReadSlow(char*) decreased pos()";
RIEGELI_ASSERT_LE(pos() - pos_before, length)
<< "Reader::ReadSlow(char*) read more than requested";
if (ABSL_PREDICT_FALSE(!read_ok)) {
*length_read = IntCast<size_t>(pos() - pos_before);
length_read = IntCast<size_t>(pos() - pos_before);
return false;
}
RIEGELI_ASSERT_EQ(pos() - pos_before, length)
<< "Reader::ReadSlow(char*) succeeded but read less than requested";
*length_read = length;
length_read = length;
return true;
}

bool Reader::ReadSlow(size_t length, std::string& dest) {
RIEGELI_ASSERT_LT(available(), length)
<< "Failed precondition of Reader::ReadSlow(string&): "
"enough data available, use Read(string&) instead";
RIEGELI_ASSERT_LE(length, dest.max_size() - dest.size())
RIEGELI_CHECK_LE(length, dest.max_size() - dest.size())
<< "Failed precondition of Reader::ReadSlow(string&): "
"string size overflow";
const size_t dest_pos = dest.size();
ResizeStringAmortized(dest, dest_pos + length);
size_t length_read;
if (ABSL_PREDICT_FALSE(!ReadSlow(length, &dest[dest_pos], &length_read))) {
if (ABSL_PREDICT_FALSE(!ReadSlow(length, &dest[dest_pos], length_read))) {
dest.erase(dest_pos + length_read);
return false;
}
return true;
}

bool Reader::ReadSlow(size_t length, std::string& dest, size_t* length_read) {
bool Reader::ReadSlow(size_t length, std::string& dest, size_t& length_read) {
RIEGELI_ASSERT_LT(available(), length)
<< "Failed precondition of Reader::ReadSlow(string&): "
"enough data available, use Read(string&) instead";
RIEGELI_ASSERT_LE(length, dest.max_size() - dest.size())
<< "Failed precondition of Reader::ReadSlow(string&): "
"string size overflow";
if (length_read == nullptr) return ReadSlow(length, dest);
const Position pos_before = pos();
const bool read_ok = ReadSlow(length, dest);
RIEGELI_ASSERT_GE(pos(), pos_before)
<< "Reader::ReadSlow(string&) decreased pos()";
RIEGELI_ASSERT_LE(pos() - pos_before, length)
<< "Reader::ReadSlow(string&) read more than requested";
if (ABSL_PREDICT_FALSE(!read_ok)) {
*length_read = IntCast<size_t>(pos() - pos_before);
length_read = IntCast<size_t>(pos() - pos_before);
return false;
}
RIEGELI_ASSERT_EQ(pos() - pos_before, length)
<< "Reader::ReadSlow(string&) succeeded but read less than requested";
*length_read = length;
length_read = length;
return true;
}

bool Reader::ReadSlowWithSizeCheck(size_t length, Chain& dest) {
RIEGELI_CHECK_LE(length, std::numeric_limits<size_t>::max() - dest.size())
<< "Failed precondition of Reader::ReadAndAppend(Chain&): "
"Chain size overflow";
return ReadSlow(length, dest);
}

bool Reader::ReadSlow(size_t length, Chain& dest) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), length)
<< "Failed precondition of Reader::ReadSlow(Chain&): "
Expand All @@ -170,27 +175,34 @@ bool Reader::ReadSlow(size_t length, Chain& dest) {
return true;
}

bool Reader::ReadSlow(size_t length, Chain& dest, size_t* length_read) {
bool Reader::ReadSlowWithSizeCheck(size_t length, Chain& dest,
size_t& length_read) {
RIEGELI_CHECK_LE(length, std::numeric_limits<size_t>::max() - dest.size())
<< "Failed precondition of Reader::ReadAndAppend(Chain&): "
"Chain size overflow";
return ReadSlow(length, dest, length_read);
}

bool Reader::ReadSlow(size_t length, Chain& dest, size_t& length_read) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), length)
<< "Failed precondition of Reader::ReadSlow(Chain&): "
"enough data available, use Read(Chain&) instead";
RIEGELI_ASSERT_LE(length, std::numeric_limits<size_t>::max() - dest.size())
<< "Failed precondition of Reader::ReadSlow(Chain&): "
"Chain size overflow";
if (length_read == nullptr) return ReadSlow(length, dest);
const Position pos_before = pos();
const bool read_ok = ReadSlow(length, dest);
RIEGELI_ASSERT_GE(pos(), pos_before)
<< "Reader::ReadSlow(Chain&) decreased pos()";
RIEGELI_ASSERT_LE(pos() - pos_before, length)
<< "Reader::ReadSlow(Chain&) read more than requested";
if (ABSL_PREDICT_FALSE(!read_ok)) {
*length_read = IntCast<size_t>(pos() - pos_before);
length_read = IntCast<size_t>(pos() - pos_before);
return false;
}
RIEGELI_ASSERT_EQ(pos() - pos_before, length)
<< "Reader::ReadSlow(Chain&) succeeded but read less than requested";
*length_read = length;
length_read = length;
return true;
}

Expand Down Expand Up @@ -259,6 +271,13 @@ inline bool ReadSlowToCord(Reader& src, size_t length, DependentCord& dest) {

} // namespace

bool Reader::ReadSlowWithSizeCheck(size_t length, absl::Cord& dest) {
RIEGELI_CHECK_LE(length, std::numeric_limits<size_t>::max() - dest.size())
<< "Failed precondition of Reader::ReadAndAppend(Cord&): "
"Cord size overflow";
return ReadSlow(length, dest);
}

bool Reader::ReadSlow(size_t length, absl::Cord& dest) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), length)
<< "Failed precondition of Reader::ReadSlow(Cord&): "
Expand All @@ -269,27 +288,34 @@ bool Reader::ReadSlow(size_t length, absl::Cord& dest) {
return ReadSlowToCord(*this, length, dest);
}

bool Reader::ReadSlow(size_t length, absl::Cord& dest, size_t* length_read) {
bool Reader::ReadSlowWithSizeCheck(size_t length, absl::Cord& dest,
size_t& length_read) {
RIEGELI_CHECK_LE(length, std::numeric_limits<size_t>::max() - dest.size())
<< "Failed precondition of Reader::ReadAndAppend(Cord&): "
"Cord size overflow";
return ReadSlow(length, dest, length_read);
}

bool Reader::ReadSlow(size_t length, absl::Cord& dest, size_t& length_read) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), length)
<< "Failed precondition of Reader::ReadSlow(Cord&): "
"enough data available, use Read(Cord&) instead";
RIEGELI_ASSERT_LE(length, std::numeric_limits<size_t>::max() - dest.size())
<< "Failed precondition of Reader::ReadSlow(Cord&): "
"Cord size overflow";
if (length_read == nullptr) return ReadSlow(length, dest);
const Position pos_before = pos();
const bool read_ok = ReadSlow(length, dest);
RIEGELI_ASSERT_GE(pos(), pos_before)
<< "Reader::ReadSlow(Cord&) decreased pos()";
RIEGELI_ASSERT_LE(pos() - pos_before, length)
<< "Reader::ReadSlow(Cord&) read more than requested";
if (ABSL_PREDICT_FALSE(!read_ok)) {
*length_read = IntCast<size_t>(pos() - pos_before);
length_read = IntCast<size_t>(pos() - pos_before);
return false;
}
RIEGELI_ASSERT_EQ(pos() - pos_before, length)
<< "Reader::ReadSlow(Cord&) succeeded but read less than requested";
*length_read = length;
length_read = length;
return true;
}

Expand All @@ -309,24 +335,23 @@ bool Reader::CopySlow(Position length, Writer& dest) {
return dest.Write(data);
}

bool Reader::CopySlow(Position length, Writer& dest, Position* length_read) {
bool Reader::CopySlow(Position length, Writer& dest, Position& length_read) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), length)
<< "Failed precondition of Reader::CopySlow(Writer&): "
"enough data available, use Copy(Writer&) instead";
if (length_read == nullptr) return CopySlow(length, dest);
const Position pos_before = pos();
const bool copy_ok = CopySlow(length, dest);
RIEGELI_ASSERT_GE(pos(), pos_before)
<< "Reader::CopySlow(Writer&) decreased pos()";
RIEGELI_ASSERT_LE(pos() - pos_before, length)
<< "Reader::CopySlow(Writer&) read more than requested";
if (ABSL_PREDICT_FALSE(!copy_ok)) {
*length_read = pos() - pos_before;
length_read = pos() - pos_before;
return false;
}
RIEGELI_ASSERT_EQ(pos() - pos_before, length)
<< "Reader::CopySlow(Writer&) succeeded but read less than requested";
*length_read = length;
length_read = length;
return true;
}

Expand Down Expand Up @@ -371,23 +396,22 @@ bool Reader::ReadSomeSlow(size_t max_length, char* dest) {
return true;
}

bool Reader::ReadSomeSlow(size_t max_length, char* dest, size_t* length_read) {
bool Reader::ReadSomeSlow(size_t max_length, char* dest, size_t& length_read) {
RIEGELI_ASSERT_LT(available(), max_length)
<< "Failed precondition of Reader::ReadSomeSlow(char*): "
"enough data available, use ReadSome(char*) instead";
if (length_read == nullptr) return ReadSomeSlow(max_length, dest);
const Position pos_before = pos();
const bool read_ok = ReadSomeSlow(max_length, dest);
RIEGELI_ASSERT_GE(pos(), pos_before)
<< "Reader::ReadSomeSlow(char*) decreased pos()";
RIEGELI_ASSERT_LE(pos() - pos_before, max_length)
<< "Reader::ReadSomeSlow(char*) read more than requested";
*length_read = IntCast<size_t>(pos() - pos_before);
length_read = IntCast<size_t>(pos() - pos_before);
if (!read_ok) {
RIEGELI_ASSERT_EQ(*length_read, 0u)
RIEGELI_ASSERT_EQ(length_read, 0u)
<< "Reader::ReadSomeSlow(char*) failed but read some";
} else {
RIEGELI_ASSERT_GT(*length_read, 0u)
RIEGELI_ASSERT_GT(length_read, 0u)
<< "Reader::ReadSomeSlow(char*) succeeded but read none";
}
return read_ok;
Expand Down Expand Up @@ -424,23 +448,22 @@ bool Reader::ReadSomeSlow(size_t max_length, std::string& dest) {
}

bool Reader::ReadSomeSlow(size_t max_length, std::string& dest,
size_t* length_read) {
size_t& length_read) {
RIEGELI_ASSERT_LT(available(), max_length)
<< "Failed precondition of Reader::ReadSomeSlow(string&): "
"enough data available, use ReadSome(string&) instead";
if (length_read == nullptr) return ReadSomeSlow(max_length, dest);
const Position pos_before = pos();
const bool read_ok = ReadSomeSlow(max_length, dest);
RIEGELI_ASSERT_GE(pos(), pos_before)
<< "Reader::ReadSomeSlow(string&) decreased pos()";
RIEGELI_ASSERT_LE(pos() - pos_before, max_length)
<< "Reader::ReadSomeSlow(string&) read more than requested";
*length_read = IntCast<size_t>(pos() - pos_before);
length_read = IntCast<size_t>(pos() - pos_before);
if (!read_ok) {
RIEGELI_ASSERT_EQ(*length_read, 0u)
RIEGELI_ASSERT_EQ(length_read, 0u)
<< "Reader::ReadSomeSlow(string&) failed but read some";
} else {
RIEGELI_ASSERT_GT(*length_read, 0u)
RIEGELI_ASSERT_GT(length_read, 0u)
<< "Reader::ReadSomeSlow(string&) succeeded but read none";
}
return read_ok;
Expand All @@ -460,23 +483,22 @@ bool Reader::ReadSomeSlow(size_t max_length, Chain& dest) {
return ReadAndAppend(UnsignedMin(max_length, available()), dest);
}

bool Reader::ReadSomeSlow(size_t max_length, Chain& dest, size_t* length_read) {
bool Reader::ReadSomeSlow(size_t max_length, Chain& dest, size_t& length_read) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), max_length)
<< "Failed precondition of Reader::ReadSomeSlow(Chain&): "
"enough data available, use ReadSome(Chain&) instead";
if (length_read == nullptr) return ReadSomeSlow(max_length, dest);
const Position pos_before = pos();
const bool read_ok = ReadSomeSlow(max_length, dest);
RIEGELI_ASSERT_GE(pos(), pos_before)
<< "Reader::ReadSomeSlow(Chain&) decreased pos()";
RIEGELI_ASSERT_LE(pos() - pos_before, max_length)
<< "Reader::ReadSomeSlow(Chain&) read more than requested";
*length_read = IntCast<size_t>(pos() - pos_before);
length_read = IntCast<size_t>(pos() - pos_before);
if (!read_ok) {
RIEGELI_ASSERT_EQ(*length_read, 0u)
RIEGELI_ASSERT_EQ(length_read, 0u)
<< "Reader::ReadSomeSlow(Chain&) failed but read some";
} else {
RIEGELI_ASSERT_GT(*length_read, 0u)
RIEGELI_ASSERT_GT(length_read, 0u)
<< "Reader::ReadSomeSlow(Chain&) succeeded but read none";
}
return read_ok;
Expand All @@ -497,23 +519,22 @@ bool Reader::ReadSomeSlow(size_t max_length, absl::Cord& dest) {
}

bool Reader::ReadSomeSlow(size_t max_length, absl::Cord& dest,
size_t* length_read) {
size_t& length_read) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), max_length)
<< "Failed precondition of Reader::ReadSomeSlow(Cord&): "
"enough data available, use ReadSome(Cord&) instead";
if (length_read == nullptr) return ReadSomeSlow(max_length, dest);
const Position pos_before = pos();
const bool read_ok = ReadSomeSlow(max_length, dest);
RIEGELI_ASSERT_GE(pos(), pos_before)
<< "Reader::ReadSomeSlow(Cord&) decreased pos()";
RIEGELI_ASSERT_LE(pos() - pos_before, max_length)
<< "Reader::ReadSomeSlow(Cord&) read more than requested";
*length_read = IntCast<size_t>(pos() - pos_before);
length_read = IntCast<size_t>(pos() - pos_before);
if (!read_ok) {
RIEGELI_ASSERT_EQ(*length_read, 0u)
RIEGELI_ASSERT_EQ(length_read, 0u)
<< "Reader::ReadSomeSlow(Cord&) failed but read some";
} else {
RIEGELI_ASSERT_GT(*length_read, 0u)
RIEGELI_ASSERT_GT(length_read, 0u)
<< "Reader::ReadSomeSlow(Cord&) succeeded but read none";
}
return read_ok;
Expand Down Expand Up @@ -544,25 +565,24 @@ bool Reader::CopySomeSlow(size_t max_length, Writer& dest) {
}

bool Reader::CopySomeSlow(size_t max_length, Writer& dest,
size_t* length_read) {
size_t& length_read) {
RIEGELI_ASSERT_LT(UnsignedMin(available(), kMaxBytesToCopy), max_length)
<< "Failed precondition of Reader::CopySomeSlow(Writer&): "
"enough data available, use CopySome(Writer&) instead";
if (length_read == nullptr) return CopySomeSlow(max_length, dest);
const Position pos_before = pos();
const bool copy_ok = CopySomeSlow(max_length, dest);
RIEGELI_ASSERT_GE(pos(), pos_before)
<< "Reader::CopySomeSlow(Writer&) decreased pos()";
RIEGELI_ASSERT_LE(pos() - pos_before, max_length)
<< "Reader::CopySomeSlow(Writer&) read more than requested";
*length_read = IntCast<size_t>(pos() - pos_before);
length_read = IntCast<size_t>(pos() - pos_before);
if (!copy_ok) {
if (dest.ok()) {
RIEGELI_ASSERT_EQ(*length_read, 0u)
RIEGELI_ASSERT_EQ(length_read, 0u)
<< "Reader::CopySomeSlow(Writer&) failed but read some";
}
} else {
RIEGELI_ASSERT_GT(*length_read, 0u)
RIEGELI_ASSERT_GT(length_read, 0u)
<< "Reader::CopySomeSlow(Writer&) succeeded but read none";
}
return copy_ok;
Expand All @@ -581,25 +601,24 @@ bool Reader::ReadOrPullSomeSlow(size_t max_length,

bool Reader::ReadOrPullSomeSlow(size_t max_length,
absl::FunctionRef<char*(size_t&)> get_dest,
size_t* length_read) {
size_t& length_read) {
RIEGELI_ASSERT_GT(max_length, 0u)
<< "Failed precondition of Reader::ReadOrPullSomeSlow(): "
"nothing to read, use ReadOrPullSome() instead";
RIEGELI_ASSERT_EQ(available(), 0u)
<< "Failed precondition of Reader::ReadOrPullSomeSlow(): "
"some data available, use ReadOrPullSome() instead";
if (length_read == nullptr) return ReadOrPullSomeSlow(max_length, get_dest);
const Position pos_before = limit_pos();
const bool read_ok = ReadOrPullSomeSlow(max_length, get_dest);
RIEGELI_ASSERT_GE(pos(), pos_before)
<< "Reader::ReadOrPullSomeSlow() decreased pos()";
RIEGELI_ASSERT_LE(pos() - pos_before, max_length)
<< "Reader::ReadOrPullSomeSlow() read more than requested";
*length_read = IntCast<size_t>(pos() - pos_before);
length_read = IntCast<size_t>(pos() - pos_before);
if (!read_ok) {
RIEGELI_ASSERT_EQ(*length_read, 0u)
RIEGELI_ASSERT_EQ(length_read, 0u)
<< "Reader::ReadOrPullSomeSlow() failed but read some";
} else if (*length_read == 0) {
} else if (length_read == 0) {
RIEGELI_ASSERT_GT(available(), 0u)
<< "Reader::ReadOrPullSomeSlow() succeeded but "
"read none and pulled none";
Expand Down
Loading

0 comments on commit 10baed7

Please sign in to comment.