Skip to content

Commit

Permalink
Optimize seeking within the buffer in BufferedWriter.
Browse files Browse the repository at this point in the history
Allow that only if `SupportsRandomAccess()`, otherwise seeking would fail
inconsistently depending on buffering and later than the nominal seek.

This requires keeping track of the maximum position written to, if that is
larger than `cursor()`.

Cosmetics: convert `Position` to `size_t` for pointer arithmetic.
PiperOrigin-RevId: 516623130
  • Loading branch information
QrczakMK committed Mar 14, 2023
1 parent 8bfa2cb commit 1d03ff7
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 14 deletions.
1 change: 1 addition & 0 deletions riegeli/bytes/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,7 @@ cc_library(
hdrs = ["array_backward_writer.h"],
deps = [
":pushable_backward_writer",
"//riegeli/base:arithmetic",
"//riegeli/base:assert",
"//riegeli/base:dependency",
"//riegeli/base:object",
Expand Down
3 changes: 2 additions & 1 deletion riegeli/bytes/array_backward_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include "absl/base/optimization.h"
#include "absl/types/span.h"
#include "riegeli/base/arithmetic.h"
#include "riegeli/base/assert.h"
#include "riegeli/base/types.h"

Expand Down Expand Up @@ -49,7 +50,7 @@ bool ArrayBackwardWriterBase::TruncateBehindScratch(Position new_size) {
"scratch used";
if (ABSL_PREDICT_FALSE(!ok())) return false;
if (ABSL_PREDICT_FALSE(new_size > start_to_cursor())) return false;
set_cursor(start() - new_size);
set_cursor(start() - IntCast<size_t>(new_size));
return true;
}

Expand Down
6 changes: 3 additions & 3 deletions riegeli/bytes/array_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ bool ArrayWriterBase::SeekBehindScratch(Position new_pos) {
return false;
}
written_ = absl::MakeSpan(start(), size);
set_cursor(start() + new_pos);
set_cursor(start() + IntCast<size_t>(new_pos));
return true;
}

Expand All @@ -91,8 +91,8 @@ bool ArrayWriterBase::TruncateBehindScratch(Position new_size) {
set_cursor(start() + size);
return false;
}
written_ = absl::MakeSpan(start(), new_size);
set_cursor(start() + new_size);
written_ = absl::MakeSpan(start(), IntCast<size_t>(new_size));
set_cursor(start() + IntCast<size_t>(new_size));
return true;
}

Expand Down
44 changes: 35 additions & 9 deletions riegeli/bytes/buffered_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,15 @@
namespace riegeli {

void BufferedWriter::Done() {
const absl::string_view src(start(), start_to_cursor());
const absl::string_view src(start(),
UnsignedMax(start_to_cursor(), written_));
const Position new_pos = pos();
set_buffer();
written_ = 0;
DoneBehindBuffer(src);
if (ABSL_PREDICT_FALSE(start_pos() != new_pos) && ABSL_PREDICT_TRUE(ok())) {
SeekBehindBuffer(new_pos);
}
Writer::Done();
buffer_ = Buffer();
}
Expand All @@ -50,11 +56,18 @@ void BufferedWriter::DoneBehindBuffer(absl::string_view src) {
}

inline bool BufferedWriter::SyncBuffer() {
const absl::string_view data(start(), start_to_cursor());
const absl::string_view data(start(),
UnsignedMax(start_to_cursor(), written_));
const Position new_pos = pos();
set_buffer();
written_ = 0;
if (data.empty()) return true;
if (ABSL_PREDICT_FALSE(!ok())) return false;
return WriteInternal(data);
if (ABSL_PREDICT_FALSE(!WriteInternal(data))) return false;
if (ABSL_PREDICT_FALSE(start_pos() != new_pos)) {
return SeekBehindBuffer(new_pos);
}
return true;
}

void BufferedWriter::SetWriteSizeHintImpl(
Expand Down Expand Up @@ -149,10 +162,16 @@ bool BufferedWriter::WriteZerosSlow(Position length) {
}

bool BufferedWriter::FlushImpl(FlushType flush_type) {
buffer_sizer_.EndRun(pos());
const absl::string_view src(start(), start_to_cursor());
buffer_sizer_.EndRun(start_pos() + UnsignedMax(start_to_cursor(), written_));
const absl::string_view src(start(),
UnsignedMax(start_to_cursor(), written_));
const Position new_pos = pos();
set_buffer();
written_ = 0;
if (ABSL_PREDICT_FALSE(!FlushBehindBuffer(src, flush_type))) return false;
if (ABSL_PREDICT_FALSE(start_pos() != new_pos)) {
if (ABSL_PREDICT_FALSE(!SeekBehindBuffer(new_pos))) return false;
}
buffer_sizer_.BeginRun(start_pos());
return true;
}
Expand All @@ -161,15 +180,22 @@ bool BufferedWriter::SeekSlow(Position new_pos) {
RIEGELI_ASSERT_NE(new_pos, pos())
<< "Failed precondition of Writer::SeekSlow(): "
"position unchanged, use Seek() instead";
buffer_sizer_.EndRun(pos());
if (ABSL_PREDICT_TRUE(
SupportsRandomAccess() && new_pos >= start_pos() &&
new_pos <= start_pos() + UnsignedMax(start_to_cursor(), written_))) {
written_ = UnsignedMax(start_to_cursor(), written_);
set_cursor(start() + IntCast<size_t>(new_pos - start_pos()));
return true;
}
buffer_sizer_.EndRun(start_pos() + UnsignedMax(start_to_cursor(), written_));
if (ABSL_PREDICT_FALSE(!SyncBuffer())) return false;
if (ABSL_PREDICT_FALSE(!SeekBehindBuffer(new_pos))) return false;
buffer_sizer_.BeginRun(start_pos());
return true;
}

absl::optional<Position> BufferedWriter::SizeImpl() {
buffer_sizer_.EndRun(pos());
buffer_sizer_.EndRun(start_pos() + UnsignedMax(start_to_cursor(), written_));
if (ABSL_PREDICT_FALSE(!SyncBuffer())) return absl::nullopt;
const absl::optional<Position> size = SizeBehindBuffer();
if (ABSL_PREDICT_FALSE(size == absl::nullopt)) return absl::nullopt;
Expand All @@ -178,15 +204,15 @@ absl::optional<Position> BufferedWriter::SizeImpl() {
}

bool BufferedWriter::TruncateImpl(Position new_size) {
buffer_sizer_.EndRun(pos());
buffer_sizer_.EndRun(start_pos() + UnsignedMax(start_to_cursor(), written_));
if (ABSL_PREDICT_FALSE(!SyncBuffer())) return false;
if (ABSL_PREDICT_FALSE(!TruncateBehindBuffer(new_size))) return false;
buffer_sizer_.BeginRun(start_pos());
return true;
}

Reader* BufferedWriter::ReadModeImpl(Position initial_pos) {
buffer_sizer_.EndRun(pos());
buffer_sizer_.EndRun(start_pos() + UnsignedMax(start_to_cursor(), written_));
if (ABSL_PREDICT_FALSE(!SyncBuffer())) return nullptr;
Reader* const reader = ReadModeBehindBuffer(initial_pos);
if (ABSL_PREDICT_FALSE(reader == nullptr)) return nullptr;
Expand Down
8 changes: 7 additions & 1 deletion riegeli/bytes/buffered_writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ class BufferedWriter : public Writer {
// Contains buffered data, to be written directly after the physical
// destination position which is `start_pos()`.
Buffer buffer_;
// Size of buffered data is `UnsignedMax(start_to_cursor(), written_)`.
size_t written_ = 0;
};

// Implementation details follow.
Expand All @@ -148,25 +150,29 @@ inline BufferedWriter::BufferedWriter(
inline BufferedWriter::BufferedWriter(BufferedWriter&& that) noexcept
: Writer(static_cast<Writer&&>(that)),
buffer_sizer_(that.buffer_sizer_),
buffer_(std::move(that.buffer_)) {}
buffer_(std::move(that.buffer_)),
written_(that.written_) {}

inline BufferedWriter& BufferedWriter::operator=(
BufferedWriter&& that) noexcept {
Writer::operator=(static_cast<Writer&&>(that));
buffer_sizer_ = that.buffer_sizer_;
buffer_ = std::move(that.buffer_);
written_ = that.written_;
return *this;
}

inline void BufferedWriter::Reset(Closed) {
Writer::Reset(kClosed);
buffer_sizer_.Reset();
buffer_ = Buffer();
written_ = 0;
}

inline void BufferedWriter::Reset(const BufferOptions& buffer_options) {
Writer::Reset();
buffer_sizer_.Reset(buffer_options);
written_ = 0;
}

} // namespace riegeli
Expand Down

0 comments on commit 1d03ff7

Please sign in to comment.