Chiebot-Mirror
/
grpc
mirror of https://github.com/grpc/grpc.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Stop N**2 symbol name lengths (#28502)

Browse Source 
* Experiment: Try to stop N**2 symbol name lengths

* Better copysink

* clean up tests

* fix clang-tidy

* fix

* typo?

* fix

* comment

* Automated change: Fix sanity tests

Co-authored-by: ctiller <ctiller@users.noreply.github.com>
pull/28560/head
Craig Tiller 3 years ago committed by GitHub
parent e0a3a513a9
commit 17859fb6b5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 118 additions and 79 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 174
      src/core/lib/transport/metadata_batch.h
  2. 4
      test/core/promise/try_seq_metadata_test.cc
  3. 19
      test/core/transport/metadata_map_test.cc

174
src/core/lib/transport/metadata_batch.h
Unescape View File

@ -739,6 +739,30 @@ struct Value<Which, absl::enable_if_t<Which::kRepeatable == true, void>> {
StorageType value;
};
// Encoder to copy some metadata
template <typename Output>
class CopySink {
public:
explicit CopySink(Output* dst) : dst_(dst) {}
template <class T, class V>
void Encode(T trait, V value) {
dst_->Set(trait, value);
}
template <class T>
void Encode(T trait, const Slice& value) {
dst_->Set(trait, std::move(value.AsOwned()));
}
void Encode(const Slice& key, const Slice& value) {
dst_->AppendUnknown(key.as_string_view(), value.Ref());
}
private:
Output* dst_;
};
} // namespace metadata_detail
// Helper function for encoders
@ -757,18 +781,28 @@ MetadataValueAsSlice(typename Which::ValueType value) {
}
// MetadataMap encodes the mapping of metadata keys to metadata values.
// Right now the API presented is the minimal one that will allow us to
// substitute this type for grpc_metadata_batch in a relatively easy fashion. At
// that point we'll start iterating this API into something that's ergonomic
// again, whilst minimally holding the performance bar already set (and
// hopefully improving some things).
// In the meantime, we're not going to invest much time in ephemeral API
// documentation, so if you must use one of these APIs and it's not obvious
// how, reach out to ctiller.
//
// MetadataMap takes a list of traits. Each of these trait objects defines
// one metadata field that is used by core, and so should have more specialized
// handling than just using the generic APIs.
// MetadataMap takes a derived class and list of traits. Each of these trait
// objects defines one metadata field that is used by core, and so should have
// more specialized handling than just using the generic APIs.
//
// MetadataMap is the backing type for some derived type via the curiously
// recursive template pattern. This is because many types consumed by
// MetadataMap require the container type to operate on, and many of those
// types are instantiated one per trait. A naive implementation without the
// Derived type would, for traits A,B,C, then instantiate for some
// T<Container, Trait>:
// - T<MetadataMap<A,B,C>, A>,
// - T<MetadataMap<A,B,C>, B>,
// - T<MetadataMap<A,B,C>, C>.
// Since these types ultimately need to be recorded in the .dynstr segment
// for dynamic linkers (if gRPC is linked as a static library) this would
// create O(N^2) bytes of symbols even in stripped libraries. To avoid this
// we use the derived type (e.g. grpc_metadata_batch right now) to capture
// the container type, and we would write T<grpc_metadata_batch, A>, etc...
// Note that now the container type uses a number of bytes that is independent
// of the number of traits, and so we return to a linear symbol table growth
// function.
//
// Each trait object has the following signature:
// // Traits for the grpc-xyz metadata field:
@ -815,7 +849,7 @@ MetadataValueAsSlice(typename Which::ValueType value) {
// for grpc-timeout we make the memento the timeout expressed on the wire, but
// we make the value the timestamp of when the timeout will expire (i.e. the
// deadline).
template <typename... Traits>
template <class Derived, typename... Traits>
class MetadataMap {
public:
explicit MetadataMap(Arena* arena);
@ -824,7 +858,10 @@ class MetadataMap {
MetadataMap(const MetadataMap&) = delete;
MetadataMap& operator=(const MetadataMap&) = delete;
MetadataMap(MetadataMap&&) noexcept;
MetadataMap& operator=(MetadataMap&&) noexcept;
// We never create MetadataMap directly, instead we create Derived, but we
// want to be able to move it without redeclaring this.
// NOLINTNEXTLINE(misc-unconventional-assign-operator)
Derived& operator=(MetadataMap&&) noexcept;
// Encode this metadata map into some encoder.
// For each field that is set in the MetadataMap, call
@ -910,7 +947,7 @@ class MetadataMap {
// Remove some metadata by name
void Remove(absl::string_view key) {
metadata_detail::RemoveHelper<MetadataMap> helper(this);
metadata_detail::RemoveHelper<Derived> helper(static_cast<Derived*>(this));
metadata_detail::NameLookup<Traits...>::Lookup(key, &helper);
}
@ -919,7 +956,8 @@ class MetadataMap {
// Retrieve some metadata by name
absl::optional<absl::string_view> GetStringValue(absl::string_view name,
std::string* buffer) const {
metadata_detail::GetStringValueHelper<MetadataMap> helper(this, buffer);
metadata_detail::GetStringValueHelper<Derived> helper(
static_cast<const Derived*>(this), buffer);
return metadata_detail::NameLookup<Traits...>::Lookup(name, &helper);
}
@ -958,36 +996,39 @@ class MetadataMap {
// that result.
// TODO(ctiller): key should probably be an absl::string_view.
// Once we don't care about interning anymore, make that change!
static ParsedMetadata<MetadataMap> Parse(absl::string_view key, Slice value,
uint32_t transport_size,
MetadataParseErrorFn on_error) {
metadata_detail::ParseHelper<MetadataMap> helper(value.TakeOwned(),
on_error, transport_size);
static ParsedMetadata<Derived> Parse(absl::string_view key, Slice value,
uint32_t transport_size,
MetadataParseErrorFn on_error) {
metadata_detail::ParseHelper<Derived> helper(value.TakeOwned(), on_error,
transport_size);
return metadata_detail::NameLookup<Traits...>::Lookup(key, &helper);
}
// Set a value from a parsed metadata object.
void Set(const ParsedMetadata<MetadataMap>& m) { m.SetOnContainer(this); }
void Set(const ParsedMetadata<Derived>& m) {
m.SetOnContainer(static_cast<Derived*>(this));
}
// Append a key/value pair - takes ownership of value
void Append(absl::string_view key, Slice value,
MetadataParseErrorFn on_error) {
metadata_detail::AppendHelper<MetadataMap> helper(this, value.TakeOwned(),
on_error);
metadata_detail::AppendHelper<Derived> helper(static_cast<Derived*>(this),
value.TakeOwned(), on_error);
metadata_detail::NameLookup<Traits...>::Lookup(key, &helper);
}
void Clear();
size_t TransportSize() const;
MetadataMap Copy() const;
Derived Copy() const;
bool empty() const { return table_.empty() && unknown_.empty(); }
size_t count() const { return table_.count() + unknown_.size(); }
private:
friend class metadata_detail::AppendHelper<MetadataMap>;
friend class metadata_detail::GetStringValueHelper<MetadataMap>;
friend class metadata_detail::RemoveHelper<MetadataMap>;
friend class ParsedMetadata<MetadataMap>;
friend class metadata_detail::AppendHelper<Derived>;
friend class metadata_detail::GetStringValueHelper<Derived>;
friend class metadata_detail::RemoveHelper<Derived>;
friend class metadata_detail::CopySink<Derived>;
friend class ParsedMetadata<Derived>;
template <typename Which>
using Value = metadata_detail::Value<Which>;
@ -1032,29 +1073,6 @@ class MetadataMap {
uint32_t size_ = 0;
};
// Encoder to copy some metadata
class CopySink {
public:
explicit CopySink(MetadataMap* dst) : dst_(dst) {}
template <class T, class V>
void Encode(T trait, V value) {
dst_->Set(trait, value);
}
template <class T>
void Encode(T trait, const Slice& value) {
dst_->Set(trait, std::move(value.AsOwned()));
}
void Encode(const Slice& key, const Slice& value) {
dst_->AppendUnknown(key.as_string_view(), value.Ref());
}
private:
MetadataMap* dst_;
};
// Encoder to log some metadata
class LogEncoder {
public:
@ -1109,53 +1127,56 @@ class MetadataMap {
// Ok/not-ok check for metadata maps that contain GrpcStatusMetadata, so that
// they can be used as result types for TrySeq.
template <typename... Args>
inline bool IsStatusOk(const MetadataMap<Args...>& m) {
template <typename Derived, typename... Args>
inline bool IsStatusOk(const MetadataMap<Derived, Args...>& m) {
return m.get(GrpcStatusMetadata()).value_or(GRPC_STATUS_UNKNOWN) ==
GRPC_STATUS_OK;
}
template <typename... Traits>
MetadataMap<Traits...>::MetadataMap(Arena* arena) : unknown_(arena) {}
template <typename Derived, typename... Traits>
MetadataMap<Derived, Traits...>::MetadataMap(Arena* arena) : unknown_(arena) {}
template <typename... Traits>
MetadataMap<Traits...>::MetadataMap(MetadataMap&& other) noexcept
template <typename Derived, typename... Traits>
MetadataMap<Derived, Traits...>::MetadataMap(MetadataMap&& other) noexcept
: table_(std::move(other.table_)), unknown_(std::move(other.unknown_)) {}
template <typename... Traits>
MetadataMap<Traits...>& MetadataMap<Traits...>::operator=(
// We never create MetadataMap directly, instead we create Derived, but we want
// to be able to move it without redeclaring this.
// NOLINTNEXTLINE(misc-unconventional-assign-operator)
template <typename Derived, typename... Traits>
Derived& MetadataMap<Derived, Traits...>::operator=(
MetadataMap&& other) noexcept {
table_ = std::move(other.table_);
unknown_ = std::move(other.unknown_);
return *this;
return static_cast<Derived&>(*this);
}
template <typename... Traits>
MetadataMap<Traits...>::~MetadataMap() = default;
template <typename Derived, typename... Traits>
MetadataMap<Derived, Traits...>::~MetadataMap() = default;
template <typename... Traits>
void MetadataMap<Traits...>::Clear() {
template <typename Derived, typename... Traits>
void MetadataMap<Derived, Traits...>::Clear() {
table_.ClearAll();
unknown_.Clear();
}
template <typename... Traits>
size_t MetadataMap<Traits...>::TransportSize() const {
template <typename Derived, typename... Traits>
size_t MetadataMap<Derived, Traits...>::TransportSize() const {
TransportSizeEncoder enc;
Encode(&enc);
return enc.size();
}
template <typename... Traits>
MetadataMap<Traits...> MetadataMap<Traits...>::Copy() const {
MetadataMap out(unknown_.arena());
CopySink sink(&out);
template <typename Derived, typename... Traits>
Derived MetadataMap<Derived, Traits...>::Copy() const {
Derived out(unknown_.arena());
metadata_detail::CopySink<Derived> sink(&out);
Encode(&sink);
return out;
}
template <typename... Traits>
void MetadataMap<Traits...>::Log(
template <typename Derived, typename... Traits>
void MetadataMap<Derived, Traits...>::Log(
absl::FunctionRef<void(absl::string_view, absl::string_view)> log_fn)
const {
LogEncoder enc(log_fn);
@ -1164,7 +1185,10 @@ void MetadataMap<Traits...>::Log(
} // namespace grpc_core
using grpc_metadata_batch = grpc_core::MetadataMap<
struct grpc_metadata_batch;
using grpc_metadata_batch_base = grpc_core::MetadataMap<
grpc_metadata_batch,
// Colon prefixed headers first
grpc_core::HttpPathMetadata, grpc_core::HttpAuthorityMetadata,
grpc_core::HttpMethodMetadata, grpc_core::HttpStatusMetadata,
@ -1181,4 +1205,8 @@ using grpc_metadata_batch = grpc_core::MetadataMap<
grpc_core::GrpcTagsBinMetadata, grpc_core::GrpcLbClientStatsMetadata,
grpc_core::LbCostBinMetadata, grpc_core::LbTokenMetadata>;
struct grpc_metadata_batch : public grpc_metadata_batch_base {
using grpc_metadata_batch_base::grpc_metadata_batch_base;
};
#endif /* GRPC_CORE_LIB_TRANSPORT_METADATA_BATCH_H */

4
test/core/promise/try_seq_metadata_test.cc
Unescape View File

@ -23,7 +23,9 @@ namespace grpc_core {
static auto* g_memory_allocator = new MemoryAllocator(
ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test"));
using TestMap = MetadataMap<GrpcStatusMetadata>;
struct TestMap : public MetadataMap<TestMap, GrpcStatusMetadata> {
using MetadataMap<TestMap, GrpcStatusMetadata>::MetadataMap;
};
TEST(PromiseTest, SucceedAndThenFail) {
auto arena = MakeScopedArena(1024, g_memory_allocator);

19
test/core/transport/metadata_map_test.cc
Unescape View File

@ -27,19 +27,28 @@ namespace testing {
static auto* g_memory_allocator = new MemoryAllocator(
ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test"));
struct EmptyMetadataMap : public MetadataMap<EmptyMetadataMap> {
using MetadataMap<EmptyMetadataMap>::MetadataMap;
};
struct TimeoutOnlyMetadataMap
: public MetadataMap<TimeoutOnlyMetadataMap, GrpcTimeoutMetadata> {
using MetadataMap<TimeoutOnlyMetadataMap, GrpcTimeoutMetadata>::MetadataMap;
};
TEST(MetadataMapTest, Noop) {
auto arena = MakeScopedArena(1024, g_memory_allocator);
MetadataMap<>(arena.get());
EmptyMetadataMap(arena.get());
}
TEST(MetadataMapTest, NoopWithDeadline) {
auto arena = MakeScopedArena(1024, g_memory_allocator);
MetadataMap<GrpcTimeoutMetadata>(arena.get());
TimeoutOnlyMetadataMap(arena.get());
}
TEST(MetadataMapTest, SimpleOps) {
auto arena = MakeScopedArena(1024, g_memory_allocator);
MetadataMap<GrpcTimeoutMetadata> map(arena.get());
TimeoutOnlyMetadataMap map(arena.get());
EXPECT_EQ(map.get_pointer(GrpcTimeoutMetadata()), nullptr);
EXPECT_EQ(map.get(GrpcTimeoutMetadata()), absl::nullopt);
map.Set(GrpcTimeoutMetadata(), 1234);
@ -74,7 +83,7 @@ class FakeEncoder {
TEST(MetadataMapTest, EmptyEncodeTest) {
FakeEncoder encoder;
auto arena = MakeScopedArena(1024, g_memory_allocator);
MetadataMap<GrpcTimeoutMetadata> map(arena.get());
TimeoutOnlyMetadataMap map(arena.get());
map.Encode(&encoder);
EXPECT_EQ(encoder.output(), "");
}
@ -82,7 +91,7 @@ TEST(MetadataMapTest, EmptyEncodeTest) {
TEST(MetadataMapTest, TimeoutEncodeTest) {
FakeEncoder encoder;
auto arena = MakeScopedArena(1024, g_memory_allocator);
MetadataMap<GrpcTimeoutMetadata> map(arena.get());
TimeoutOnlyMetadataMap map(arena.get());
map.Set(GrpcTimeoutMetadata(), 1234);
map.Encode(&encoder);
EXPECT_EQ(encoder.output(), "grpc-timeout: deadline=1234\n");

Write Preview
Loading…
Cancel
Save