From add2e2c076d493885f9facbeb32cadc492743315 Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Mon, 16 Jan 2023 16:46:18 +0100 Subject: [PATCH] LibCore: Do short forward seeks by discarding bytes from the buffer This saves us an actual seek and rereading already stored buffer data in cases where the seek is entirely covered by the currently buffered data. This is especially important since we implement `discard` using `seek` for seekable streams. --- Tests/LibCore/TestLibCoreStream.cpp | 19 +++++++++++++++++-- Userland/Libraries/LibCore/Stream.h | 14 +++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/Tests/LibCore/TestLibCoreStream.cpp b/Tests/LibCore/TestLibCoreStream.cpp index 48fb655ec8..bf256814cc 100644 --- a/Tests/LibCore/TestLibCoreStream.cpp +++ b/Tests/LibCore/TestLibCoreStream.cpp @@ -470,8 +470,9 @@ TEST_CASE(buffered_small_file_read) TEST_CASE(buffered_file_tell_and_seek) { + // We choose a buffer size of 12 bytes to cover half of the input file. auto file = Core::Stream::File::open("/usr/Tests/LibCore/small.txt"sv, Core::Stream::OpenMode::Read).release_value(); - auto buffered_file = Core::Stream::BufferedFile::create(move(file)).release_value(); + auto buffered_file = Core::Stream::BufferedFile::create(move(file), 12).release_value(); // Initial state. { @@ -529,13 +530,27 @@ TEST_CASE(buffered_file_tell_and_seek) EXPECT_EQ(current_offset, 0); } - // Read the first character. + // Read the first character. This should prime the buffer if it hasn't happened already. { auto character = buffered_file->read_value().release_value(); EXPECT_EQ(character, 'W'); auto current_offset = buffered_file->tell().release_value(); EXPECT_EQ(current_offset, 1); } + + // Seek beyond the buffer size, which should invalidate the buffer. + { + auto current_offset = buffered_file->seek(12, Core::Stream::SeekMode::SetPosition).release_value(); + EXPECT_EQ(current_offset, 12); + } + + // Ensure that we still read the correct contents from the new offset with a (presumably) freshly filled buffer. + { + auto character = buffered_file->read_value().release_value(); + EXPECT_EQ(character, 'r'); + auto current_offset = buffered_file->tell().release_value(); + EXPECT_EQ(current_offset, 13); + } } constexpr auto buffered_sent_data = "Well hello friends!\n:^)\nThis shouldn't be present. :^("sv; diff --git a/Userland/Libraries/LibCore/Stream.h b/Userland/Libraries/LibCore/Stream.h index e17d24184f..46949d45b7 100644 --- a/Userland/Libraries/LibCore/Stream.h +++ b/Userland/Libraries/LibCore/Stream.h @@ -819,6 +819,11 @@ public: m_buffer.clear(); } + ErrorOr discard_bytes(size_t count) + { + return m_buffer.discard(count); + } + private: ErrorOr populate_read_buffer() { @@ -875,8 +880,15 @@ public: virtual void close() override { m_helper.stream().close(); } virtual ErrorOr seek(i64 offset, SeekMode mode) override { - if (mode == SeekMode::FromCurrentPosition) + if (mode == SeekMode::FromCurrentPosition) { + // If possible, seek using the buffer alone. + if (0 <= offset && static_cast(offset) <= m_helper.buffered_data_size()) { + MUST(m_helper.discard_bytes(offset)); + return TRY(m_helper.stream().tell()) - m_helper.buffered_data_size(); + } + offset = offset - m_helper.buffered_data_size(); + } auto result = TRY(m_helper.stream().seek(offset, mode)); m_helper.clear_buffer();