Commit 9a79278a by Matt Kulukundis Committed by Copybara-Service

Fix a corner case in SpyHashState for exact boundaries.

AbslHash allows for piecewise chunks to be streamed incrementally into hash states and requires them to hash identically to one giant stream.  The exact size window for this is an internal details `PiecewiseChunkSize`.  There was an off by one error in this code.  Add tests and fix it.

PiperOrigin-RevId: 602463183
Change-Id: I159bbb5e7e745f55b2fe6eaf0d2735bd0a08aca9
parent 42624b3d
...@@ -149,20 +149,20 @@ class SpyHashStateImpl : public HashStateBase<SpyHashStateImpl<T>> { ...@@ -149,20 +149,20 @@ class SpyHashStateImpl : public HashStateBase<SpyHashStateImpl<T>> {
const unsigned char* begin, const unsigned char* begin,
size_t size) { size_t size) {
const size_t large_chunk_stride = PiecewiseChunkSize(); const size_t large_chunk_stride = PiecewiseChunkSize();
if (size > large_chunk_stride) { // Combining a large contiguous buffer must have the same effect as
// Combining a large contiguous buffer must have the same effect as // doing it piecewise by the stride length, followed by the (possibly
// doing it piecewise by the stride length, followed by the (possibly // empty) remainder.
// empty) remainder. while (size > large_chunk_stride) {
while (size >= large_chunk_stride) { hash_state = SpyHashStateImpl::combine_contiguous(
hash_state = SpyHashStateImpl::combine_contiguous( std::move(hash_state), begin, large_chunk_stride);
std::move(hash_state), begin, large_chunk_stride); begin += large_chunk_stride;
begin += large_chunk_stride; size -= large_chunk_stride;
size -= large_chunk_stride;
}
} }
hash_state.hash_representation_.emplace_back( if (size > 0) {
reinterpret_cast<const char*>(begin), size); hash_state.hash_representation_.emplace_back(
reinterpret_cast<const char*>(begin), size);
}
return hash_state; return hash_state;
} }
......
...@@ -918,6 +918,7 @@ cc_test( ...@@ -918,6 +918,7 @@ cc_test(
"//absl/container:fixed_array", "//absl/container:fixed_array",
"//absl/functional:function_ref", "//absl/functional:function_ref",
"//absl/hash", "//absl/hash",
"//absl/hash:hash_testing",
"//absl/log", "//absl/log",
"//absl/log:check", "//absl/log:check",
"//absl/random", "//absl/random",
......
...@@ -1072,6 +1072,7 @@ absl_cc_test( ...@@ -1072,6 +1072,7 @@ absl_cc_test(
absl::fixed_array absl::fixed_array
absl::function_ref absl::function_ref
absl::hash absl::hash
absl::hash_testing
absl::log absl::log
absl::optional absl::optional
absl::random_random absl::random_random
......
...@@ -42,6 +42,7 @@ ...@@ -42,6 +42,7 @@
#include "absl/container/fixed_array.h" #include "absl/container/fixed_array.h"
#include "absl/functional/function_ref.h" #include "absl/functional/function_ref.h"
#include "absl/hash/hash.h" #include "absl/hash/hash.h"
#include "absl/hash/hash_testing.h"
#include "absl/log/check.h" #include "absl/log/check.h"
#include "absl/log/log.h" #include "absl/log/log.h"
#include "absl/random/random.h" #include "absl/random/random.h"
...@@ -2013,6 +2014,26 @@ TEST(CordTest, CordMemoryUsageBTree) { ...@@ -2013,6 +2014,26 @@ TEST(CordTest, CordMemoryUsageBTree) {
rep2_size); rep2_size);
} }
TEST(CordTest, TestHashFragmentation) {
// Make sure we hit these boundary cases precisely.
EXPECT_EQ(1024, absl::hash_internal::PiecewiseChunkSize());
EXPECT_TRUE(absl::VerifyTypeImplementsAbslHashCorrectly({
absl::Cord(),
absl::MakeFragmentedCord({std::string(600, 'a'), std::string(600, 'a')}),
absl::MakeFragmentedCord({std::string(1200, 'a')}),
absl::MakeFragmentedCord({std::string(900, 'b'), std::string(900, 'b')}),
absl::MakeFragmentedCord({std::string(1800, 'b')}),
absl::MakeFragmentedCord(
{std::string(2000, 'c'), std::string(2000, 'c')}),
absl::MakeFragmentedCord({std::string(4000, 'c')}),
absl::MakeFragmentedCord({std::string(1024, 'd')}),
absl::MakeFragmentedCord({std::string(1023, 'd'), "d"}),
absl::MakeFragmentedCord({std::string(1025, 'e')}),
absl::MakeFragmentedCord({std::string(1024, 'e'), "e"}),
absl::MakeFragmentedCord({std::string(1023, 'e'), "e", "e"}),
}));
}
// Regtest for a change that had to be rolled back because it expanded out // Regtest for a change that had to be rolled back because it expanded out
// of the InlineRep too soon, which was observable through MemoryUsage(). // of the InlineRep too soon, which was observable through MemoryUsage().
TEST_P(CordTest, CordMemoryUsageInlineRep) { TEST_P(CordTest, CordMemoryUsageInlineRep) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment