From df60c82df43e33274550e758c7a93fa49f88e0fe Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Mon, 20 Apr 2020 07:20:27 -0700 Subject: [PATCH] Export of internal Abseil changes -- b885a238ec13effcc407e250583e293052bd7984 by Greg Falcon : Remove the dependency of //absl/hash on //absl/strings:cord. The `AbslHashValue` definition should reside in cord.h, but the implementation currently needs internal details from the hash library. This CL changes the way that Cord gains access to those internals. Note that PiecewiseCombiner remains an internal namespace API, and we still reserve the right to make changes to it. The cord_benchmark shows no statistically significant changes in hash performance with this change. PiperOrigin-RevId: 307393448 -- ca449f230ee719d069d9217ba28a07bf5b3bd8b1 by Derek Mauro : Move the extension to use absl::Format() with absl::Cord as a sink to cord.h PiperOrigin-RevId: 307077162 GitOrigin-RevId: b885a238ec13effcc407e250583e293052bd7984 Change-Id: If24a90782c786fa0b4343bc7d72d053b66c153ea --- absl/hash/BUILD.bazel | 1 - absl/hash/CMakeLists.txt | 1 - absl/hash/internal/hash.h | 135 ++++++++---------- absl/strings/BUILD.bazel | 2 +- absl/strings/CMakeLists.txt | 2 +- absl/strings/cord.h | 31 +++- absl/strings/cord_test.cc | 9 ++ .../internal/str_format/extension_test.cc | 9 -- absl/strings/internal/str_format/output.h | 9 -- 9 files changed, 92 insertions(+), 107 deletions(-) diff --git a/absl/hash/BUILD.bazel b/absl/hash/BUILD.bazel index 59eac784..6c77f1a1 100644 --- a/absl/hash/BUILD.bazel +++ b/absl/hash/BUILD.bazel @@ -43,7 +43,6 @@ cc_library( "//absl/meta:type_traits", "//absl/numeric:int128", "//absl/strings", - "//absl/strings:cord", "//absl/types:optional", "//absl/types:variant", "//absl/utility", diff --git a/absl/hash/CMakeLists.txt b/absl/hash/CMakeLists.txt index 4e555147..61365e9b 100644 --- a/absl/hash/CMakeLists.txt +++ b/absl/hash/CMakeLists.txt @@ -25,7 +25,6 @@ absl_cc_library( COPTS ${ABSL_DEFAULT_COPTS} DEPS - absl::cord absl::core_headers absl::endian absl::fixed_array diff --git a/absl/hash/internal/hash.h b/absl/hash/internal/hash.h index 025d287f..a71bd4a6 100644 --- a/absl/hash/internal/hash.h +++ b/absl/hash/internal/hash.h @@ -43,7 +43,6 @@ #include "absl/container/fixed_array.h" #include "absl/meta/type_traits.h" #include "absl/numeric/int128.h" -#include "absl/strings/cord.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "absl/types/variant.h" @@ -54,12 +53,65 @@ namespace absl { ABSL_NAMESPACE_BEGIN namespace hash_internal { -class PiecewiseCombiner; - // Internal detail: Large buffers are hashed in smaller chunks. This function // returns the size of these chunks. constexpr size_t PiecewiseChunkSize() { return 1024; } +// PiecewiseCombiner +// +// PiecewiseCombiner is an internal-only helper class for hashing a piecewise +// buffer of `char` or `unsigned char` as though it were contiguous. This class +// provides two methods: +// +// H add_buffer(state, data, size) +// H finalize(state) +// +// `add_buffer` can be called zero or more times, followed by a single call to +// `finalize`. This will produce the same hash expansion as concatenating each +// buffer piece into a single contiguous buffer, and passing this to +// `H::combine_contiguous`. +// +// Example usage: +// PiecewiseCombiner combiner; +// for (const auto& piece : pieces) { +// state = combiner.add_buffer(std::move(state), piece.data, piece.size); +// } +// return combiner.finalize(std::move(state)); +class PiecewiseCombiner { + public: + PiecewiseCombiner() : position_(0) {} + PiecewiseCombiner(const PiecewiseCombiner&) = delete; + PiecewiseCombiner& operator=(const PiecewiseCombiner&) = delete; + + // PiecewiseCombiner::add_buffer() + // + // Appends the given range of bytes to the sequence to be hashed, which may + // modify the provided hash state. + template + H add_buffer(H state, const unsigned char* data, size_t size); + template + H add_buffer(H state, const char* data, size_t size) { + return add_buffer(std::move(state), + reinterpret_cast(data), size); + } + + // PiecewiseCombiner::finalize() + // + // Finishes combining the hash sequence, which may may modify the provided + // hash state. + // + // Once finalize() is called, add_buffer() may no longer be called. The + // resulting hash state will be the same as if the pieces passed to + // add_buffer() were concatenated into a single flat buffer, and then provided + // to H::combine_contiguous(). + template + H finalize(H state); + + private: + unsigned char buf_[PiecewiseChunkSize()]; + size_t position_; +}; + // HashStateBase // // A hash state object represents an intermediate state in the computation @@ -126,8 +178,7 @@ class HashStateBase { template static H combine_contiguous(H state, const T* data, size_t size); - private: - friend class PiecewiseCombiner; + using AbslInternalPiecewiseCombiner = PiecewiseCombiner; }; // is_uniquely_represented @@ -198,61 +249,6 @@ H hash_bytes(H hash_state, const T& value) { return H::combine_contiguous(std::move(hash_state), start, sizeof(value)); } -// PiecewiseCombiner -// -// PiecewiseCombiner is an internal-only helper class for hashing a piecewise -// buffer of `char` or `unsigned char` as though it were contiguous. This class -// provides two methods: -// -// H add_buffer(state, data, size) -// H finalize(state) -// -// `add_buffer` can be called zero or more times, followed by a single call to -// `finalize`. This will produce the same hash expansion as concatenating each -// buffer piece into a single contiguous buffer, and passing this to -// `H::combine_contiguous`. -// -// Example usage: -// PiecewiseCombiner combiner; -// for (const auto& piece : pieces) { -// state = combiner.add_buffer(std::move(state), piece.data, piece.size); -// } -// return combiner.finalize(std::move(state)); -class PiecewiseCombiner { - public: - PiecewiseCombiner() : position_(0) {} - PiecewiseCombiner(const PiecewiseCombiner&) = delete; - PiecewiseCombiner& operator=(const PiecewiseCombiner&) = delete; - - // PiecewiseCombiner::add_buffer() - // - // Appends the given range of bytes to the sequence to be hashed, which may - // modify the provided hash state. - template - H add_buffer(H state, const unsigned char* data, size_t size); - template - H add_buffer(H state, const char* data, size_t size) { - return add_buffer(std::move(state), - reinterpret_cast(data), size); - } - - // PiecewiseCombiner::finalize() - // - // Finishes combining the hash sequence, which may may modify the provided - // hash state. - // - // Once finalize() is called, add_buffer() may no longer be called. The - // resulting hash state will be the same as if the pieces passed to - // add_buffer() were concatenated into a single flat buffer, and then provided - // to H::combine_contiguous(). - template - H finalize(H state); - - private: - unsigned char buf_[PiecewiseChunkSize()]; - size_t position_; -}; - // ----------------------------------------------------------------------------- // AbslHashValue for Basic Types // ----------------------------------------------------------------------------- @@ -443,25 +439,6 @@ H AbslHashValue( str.size()); } -template -H HashFragmentedCord(H hash_state, const absl::Cord& c) { - PiecewiseCombiner combiner; - c.ForEachChunk([&combiner, &hash_state](absl::string_view chunk) { - hash_state = - combiner.add_buffer(std::move(hash_state), chunk.data(), chunk.size()); - }); - return H::combine(combiner.finalize(std::move(hash_state)), c.size()); -} - -template -H AbslHashValue(H hash_state, const absl::Cord& c) { - absl::optional maybe_flat = c.TryFlat(); - if (maybe_flat.has_value()) { - return H::combine(std::move(hash_state), *maybe_flat); - } - return hash_internal::HashFragmentedCord(std::move(hash_state), c); -} - // ----------------------------------------------------------------------------- // AbslHashValue for Sequence Containers // ----------------------------------------------------------------------------- diff --git a/absl/strings/BUILD.bazel b/absl/strings/BUILD.bazel index 4ee5a2ca..8aecbe59 100644 --- a/absl/strings/BUILD.bazel +++ b/absl/strings/BUILD.bazel @@ -310,6 +310,7 @@ cc_test( deps = [ ":cord", ":cord_test_helpers", + ":str_format", ":strings", "//absl/base", "//absl/base:config", @@ -667,7 +668,6 @@ cc_test( copts = ABSL_TEST_COPTS, visibility = ["//visibility:private"], deps = [ - ":cord", ":str_format", ":str_format_internal", ":strings", diff --git a/absl/strings/CMakeLists.txt b/absl/strings/CMakeLists.txt index 10213022..b6705ed0 100644 --- a/absl/strings/CMakeLists.txt +++ b/absl/strings/CMakeLists.txt @@ -425,7 +425,6 @@ absl_cc_test( DEPS absl::str_format absl::str_format_internal - absl::cord absl::strings gmock_main ) @@ -581,6 +580,7 @@ absl_cc_test( ${ABSL_TEST_COPTS} DEPS absl::cord + absl::str_format absl::strings absl::base absl::config diff --git a/absl/strings/cord.h b/absl/strings/cord.h index 2d92f6d6..9d99b2af 100644 --- a/absl/strings/cord.h +++ b/absl/strings/cord.h @@ -90,10 +90,6 @@ class CordTestPeer; template Cord MakeCordFromExternal(absl::string_view, Releaser&&); void CopyCordToString(const Cord& src, std::string* dst); -namespace hash_internal { -template -H HashFragmentedCord(H, const Cord&); -} // Cord // @@ -615,10 +611,22 @@ class Cord { // If the cord was already flat, the contents are not modified. absl::string_view Flatten(); + // Support absl::Cord as a sink object for absl::Format(). + friend void AbslFormatFlush(absl::Cord* cord, absl::string_view part) { + cord->Append(part); + } + + template + friend H AbslHashValue(H hash_state, const absl::Cord& c) { + absl::optional maybe_flat = c.TryFlat(); + if (maybe_flat.has_value()) { + return H::combine(std::move(hash_state), *maybe_flat); + } + return c.HashFragmented(std::move(hash_state)); + } + private: friend class CordTestPeer; - template - friend H absl::hash_internal::HashFragmentedCord(H, const Cord&); friend bool operator==(const Cord& lhs, const Cord& rhs); friend bool operator==(const Cord& lhs, absl::string_view rhs); @@ -763,6 +771,17 @@ class Cord { // Helper for Append() template void AppendImpl(C&& src); + + // Helper for AbslHashValue() + template + H HashFragmented(H hash_state) const { + typename H::AbslInternalPiecewiseCombiner combiner; + ForEachChunk([&combiner, &hash_state](absl::string_view chunk) { + hash_state = combiner.add_buffer(std::move(hash_state), chunk.data(), + chunk.size()); + }); + return H::combine(combiner.finalize(std::move(hash_state)), size()); + } }; ABSL_NAMESPACE_END diff --git a/absl/strings/cord_test.cc b/absl/strings/cord_test.cc index 49178498..336cedde 100644 --- a/absl/strings/cord_test.cc +++ b/absl/strings/cord_test.cc @@ -22,6 +22,7 @@ #include "absl/container/fixed_array.h" #include "absl/strings/cord_test_helpers.h" #include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" #include "absl/strings/string_view.h" typedef std::mt19937_64 RandomEngine; @@ -1582,6 +1583,14 @@ TEST(Cord, SmallBufferAssignFromOwnData) { } } +TEST(Cord, Format) { + absl::Cord c; + absl::Format(&c, "There were %04d little %s.", 3, "pigs"); + EXPECT_EQ(c, "There were 0003 little pigs."); + absl::Format(&c, "And %-3llx bad wolf!", 1); + EXPECT_EQ(c, "There were 0003 little pigs.And 1 bad wolf!"); +} + TEST(CordDeathTest, Hardening) { absl::Cord cord("hello"); // These statement should abort the program in all builds modes. diff --git a/absl/strings/internal/str_format/extension_test.cc b/absl/strings/internal/str_format/extension_test.cc index dc5576b6..561eaa36 100644 --- a/absl/strings/internal/str_format/extension_test.cc +++ b/absl/strings/internal/str_format/extension_test.cc @@ -19,7 +19,6 @@ #include #include -#include "absl/strings/cord.h" #include "gtest/gtest.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" @@ -82,14 +81,6 @@ TEST(FormatExtensionTest, SinkAppendChars) { } } -TEST(FormatExtensionTest, CordSink) { - absl::Cord c; - absl::Format(&c, "There were %04d little %s.", 3, "pigs"); - EXPECT_EQ(c, "There were 0003 little pigs."); - absl::Format(&c, "And %-3llx bad wolf!", 1); - EXPECT_EQ(c, "There were 0003 little pigs.And 1 bad wolf!"); -} - TEST(FormatExtensionTest, CustomSink) { my_namespace::UserDefinedType sink; absl::Format(&sink, "There were %04d little %s.", 3, "pigs"); diff --git a/absl/strings/internal/str_format/output.h b/absl/strings/internal/str_format/output.h index c3168d20..8030dae0 100644 --- a/absl/strings/internal/str_format/output.h +++ b/absl/strings/internal/str_format/output.h @@ -30,9 +30,6 @@ namespace absl { ABSL_NAMESPACE_BEGIN - -class Cord; - namespace str_format_internal { // RawSink implementation that writes into a char* buffer. @@ -77,12 +74,6 @@ inline void AbslFormatFlush(std::ostream* out, string_view s) { out->write(s.data(), s.size()); } -template ::value>::type> -inline void AbslFormatFlush(AbslCord* out, string_view s) { - out->Append(s); -} - inline void AbslFormatFlush(FILERawSink* sink, string_view v) { sink->Write(v); }