From 42d01b21d869ca549b660011d8f996a8eb01e803 Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Fri, 2 Jun 2023 01:25:56 +0200 Subject: [PATCH] AK: Rewrite the hint-based `CircularBuffer::find_copy_in_seekback` This now searches the memory in blocks, which should be slightly more efficient. However, it doesn't make much difference (e.g. ~1% in LZMA compression) in most real-world applications, as the non-hint function is more expensive by orders of magnitude. --- AK/CircularBuffer.cpp | 59 ++++++++++--------------- AK/CircularBuffer.h | 2 +- Tests/AK/TestCircularBuffer.cpp | 20 ++++----- Userland/Libraries/LibCompress/Lzma.cpp | 6 +-- 4 files changed, 36 insertions(+), 51 deletions(-) diff --git a/AK/CircularBuffer.cpp b/AK/CircularBuffer.cpp index 9f011d741f3..647d4520471 100644 --- a/AK/CircularBuffer.cpp +++ b/AK/CircularBuffer.cpp @@ -375,7 +375,7 @@ ErrorOr> SearchableCircularBuffer::find_ return matches; } -ErrorOr> SearchableCircularBuffer::find_copy_in_seekback(Vector const& distances, size_t maximum_length, size_t minimum_length) const +Optional SearchableCircularBuffer::find_copy_in_seekback(ReadonlySpan distances, size_t maximum_length, size_t minimum_length) const { VERIFY(minimum_length > 0); @@ -384,55 +384,42 @@ ErrorOr> SearchableCircularBuffer::find_ maximum_length = m_used_space; if (maximum_length < minimum_length) - return Vector {}; + return Optional {}; - Vector matches; + Optional best_match; + + for (auto distance : distances) { + // Discard distances outside the valid range. + if (distance > search_limit() || distance <= 0) + continue; - // Verify all hints that we have. - for (auto const& distance : distances) { // TODO: This does not yet support looping repetitions. if (distance < minimum_length) continue; - auto needle_offset = (capacity() + m_reading_head) % capacity(); - auto haystack_offset = (capacity() + m_reading_head - distance) % capacity(); + auto current_match_length = 0ul; - for (size_t i = 0; i < minimum_length; i++) { - if (m_buffer[needle_offset] != m_buffer[haystack_offset]) + while (current_match_length < maximum_length) { + auto haystack = next_search_span(distance - current_match_length).trim(maximum_length - current_match_length); + auto needle = next_read_span(current_match_length).trim(maximum_length - current_match_length); + + auto submatch_length = haystack.matching_prefix_length(needle); + + if (submatch_length == 0) break; - needle_offset = (needle_offset + 1) % capacity(); - haystack_offset = (haystack_offset + 1) % capacity(); - - if (i + 1 == minimum_length) - TRY(matches.try_empend(distance, minimum_length)); - } - } - - // From now on, all matches that we have stored have at least a length of `minimum_length` and they all refer to the same value. - // For the remaining part, we will keep checking the next byte incrementally and keep eliminating matches until we eliminated all of them. - Vector next_matches; - - for (size_t offset = minimum_length; offset < maximum_length; offset++) { - auto needle_data = m_buffer[(capacity() + m_reading_head + offset) % capacity()]; - - for (auto const& match : matches) { - auto haystack_data = m_buffer[(capacity() + m_reading_head - match.distance + offset) % capacity()]; - - if (haystack_data != needle_data) - continue; - - TRY(next_matches.try_empend(match.distance, match.length + 1)); + current_match_length += submatch_length; } - if (next_matches.size() == 0) - return matches; + // Discard matches that don't reach the minimum length. + if (current_match_length < minimum_length) + continue; - swap(matches, next_matches); - next_matches.clear_with_capacity(); + if (!best_match.has_value() || best_match->length < current_match_length) + best_match = Match { distance, current_match_length }; } - return matches; + return best_match; } } diff --git a/AK/CircularBuffer.h b/AK/CircularBuffer.h index f535b9c72c2..c9a77d78180 100644 --- a/AK/CircularBuffer.h +++ b/AK/CircularBuffer.h @@ -75,7 +75,7 @@ public: /// Supplying any hints will only consider those distances, in case existing offsets need to be validated. /// Note that, since we only start searching at the read head, the length between read head and write head is excluded from the distance. ErrorOr> find_copy_in_seekback(size_t maximum_length, size_t minimum_length = 2) const; - ErrorOr> find_copy_in_seekback(Vector const& distances, size_t maximum_length, size_t minimum_length = 2) const; + Optional find_copy_in_seekback(ReadonlySpan distances, size_t maximum_length, size_t minimum_length = 2) const; private: // Note: This function has a similar purpose as next_seekback_span, but they differ in their reference point. diff --git a/Tests/AK/TestCircularBuffer.cpp b/Tests/AK/TestCircularBuffer.cpp index 45d694e863b..43e26bf3b95 100644 --- a/Tests/AK/TestCircularBuffer.cpp +++ b/Tests/AK/TestCircularBuffer.cpp @@ -411,25 +411,23 @@ TEST_CASE(find_copy_in_seekback) } { - // Find the largest matches with a length between 1 and 2 (selected "AB", everything smaller gets eliminated). - auto matches = MUST(buffer.find_copy_in_seekback(Vector { 6ul, 9ul }, 2, 1)); - EXPECT_EQ(matches.size(), 2ul); - EXPECT_EQ(matches[0].distance, 6ul); - EXPECT_EQ(matches[0].length, 2ul); - EXPECT_EQ(matches[1].distance, 9ul); - EXPECT_EQ(matches[1].length, 2ul); + // Find the largest match with a length between 1 and 2 (selected "AB", everything smaller gets eliminated). + // Since we have a tie, the first qualified match is preferred. + auto match = buffer.find_copy_in_seekback(Vector { 6ul, 9ul }, 2, 1); + EXPECT_EQ(match.value().distance, 6ul); + EXPECT_EQ(match.value().length, 2ul); } { // Check that we don't find anything for hints before the valid range. - auto matches = MUST(buffer.find_copy_in_seekback(Vector { 0ul }, 2, 1)); - EXPECT_EQ(matches.size(), 0ul); + auto match = buffer.find_copy_in_seekback(Vector { 0ul }, 2, 1); + EXPECT(!match.has_value()); } { // Check that we don't find anything for hints after the valid range. - auto matches = MUST(buffer.find_copy_in_seekback(Vector { 12ul }, 2, 1)); - EXPECT_EQ(matches.size(), 0ul); + auto match = buffer.find_copy_in_seekback(Vector { 12ul }, 2, 1); + EXPECT(!match.has_value()); } { diff --git a/Userland/Libraries/LibCompress/Lzma.cpp b/Userland/Libraries/LibCompress/Lzma.cpp index 6dfef752804..123354e2235 100644 --- a/Userland/Libraries/LibCompress/Lzma.cpp +++ b/Userland/Libraries/LibCompress/Lzma.cpp @@ -998,10 +998,10 @@ ErrorOr LzmaCompressor::encode_once() m_rep2 + normalized_to_real_match_distance_offset, m_rep3 + normalized_to_real_match_distance_offset, }; - auto existing_distance_results = TRY(m_dictionary->find_copy_in_seekback(existing_distances, m_dictionary->used_space(), normalized_to_real_match_length_offset)); + auto existing_distance_result = m_dictionary->find_copy_in_seekback(existing_distances, m_dictionary->used_space(), normalized_to_real_match_length_offset); - if (existing_distance_results.size() > 0) { - auto selected_match = existing_distance_results[0]; + if (existing_distance_result.has_value()) { + auto selected_match = existing_distance_result.release_value(); TRY(encode_existing_match(selected_match.distance, selected_match.length)); return {}; }