From 1934daf26ee1bc8518649ce51f13e54213b48efe Mon Sep 17 00:00:00 2001 From: Martijn Vels Date: Thu, 19 Oct 2023 20:05:29 -0700 Subject: [PATCH] Internal changes PiperOrigin-RevId: 575078212 --- src/google/protobuf/compiler/cpp/helpers.h | 11 +++- .../cpp/tools/analyze_profile_proto.cc | 51 ++++++++++++------- .../cpp/tools/analyze_profile_proto_test.cc | 21 ++++---- .../tools/analyze_profile_proto_test.proto | 2 + 4 files changed, 56 insertions(+), 29 deletions(-) diff --git a/src/google/protobuf/compiler/cpp/helpers.h b/src/google/protobuf/compiler/cpp/helpers.h index 6e5896dc80..4c590cdc76 100644 --- a/src/google/protobuf/compiler/cpp/helpers.h +++ b/src/google/protobuf/compiler/cpp/helpers.h @@ -358,9 +358,18 @@ bool IsStringInliningEnabled(const Options& options); // Returns true if the provided field is a singular string and can be inlined. bool CanStringBeInlined(const FieldDescriptor* field); -// Returns true if `field` should be inlined based on PDProto profile. +// Returns true if `field` is a string field that can and should be inlined +// based on PDProto profile. bool IsStringInlined(const FieldDescriptor* field, const Options& options); +// Returns true if `field` should be inlined based on PDProto profile. +// Currently we only enable inlining for string fields backed by a std::string +// instance, but in the future we may expand this to message types. +inline bool IsFieldInlined(const FieldDescriptor* field, + const Options& options) { + return IsStringInlined(field, options); +} + // Does the given FileDescriptor use lazy fields? bool HasLazyFields(const FileDescriptor* file, const Options& options, MessageSCCAnalyzer* scc_analyzer); diff --git a/src/google/protobuf/compiler/cpp/tools/analyze_profile_proto.cc b/src/google/protobuf/compiler/cpp/tools/analyze_profile_proto.cc index f5042371b1..8af6b57380 100644 --- a/src/google/protobuf/compiler/cpp/tools/analyze_profile_proto.cc +++ b/src/google/protobuf/compiler/cpp/tools/analyze_profile_proto.cc @@ -19,6 +19,7 @@ #include "google/protobuf/testing/file.h" #include "google/protobuf/testing/file.h" #include "google/protobuf/compiler/access_info_map.h" +#include "google/protobuf/compiler/split_map.h" #include "google/protobuf/compiler/profile_bootstrap.pb.h" #include "absl/log/absl_log.h" #include "absl/log/log.h" @@ -29,6 +30,7 @@ #include "absl/strings/string_view.h" #include "google/protobuf/compiler/cpp/cpp_access_info_parse_helper.h" #include "google/protobuf/compiler/cpp/helpers.h" +#include "google/protobuf/compiler/cpp/options.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" #include "third_party/re2/re2.h" @@ -61,7 +63,7 @@ std::ostream& operator<<(std::ostream& s, PDProtoScale scale) { return s; } -enum PDProtoOptimization { kNone, kLazy, kInline, kSplit }; +enum PDProtoOptimization { kNone, kUnverifiedLazy, kLazy, kInline, kSplit }; std::ostream& operator<<(std::ostream& s, PDProtoOptimization optimization) { switch (optimization) { @@ -69,6 +71,8 @@ std::ostream& operator<<(std::ostream& s, PDProtoOptimization optimization) { return s << "NONE"; case PDProtoOptimization::kLazy: return s << "LAZY"; + case PDProtoOptimization::kUnverifiedLazy: + return s << "UNVERIFIED_LAZY"; case PDProtoOptimization::kInline: return s << "INLINE"; case PDProtoOptimization::kSplit: @@ -83,6 +87,16 @@ class PDProtoAnalyzer { : info_map_(access_info) { info_map_.SetAccessInfoParseHelper( std::make_unique()); + options_.access_info_map = &info_map_; + scc_analyzer_ = std::make_unique(options_); + } + + void SetFile(const FileDescriptor* file) { + if (current_file_ != file) { + split_map_ = cpp::CreateSplitMap(file, options_); + options_.split_map = &split_map_; + current_file_ = file; + } } bool HasProfile(const Descriptor* descriptor) const { @@ -110,25 +124,20 @@ class PDProtoAnalyzer { } PDProtoOptimization OptimizeField(const FieldDescriptor* field) { - PDProtoAnalysis analysis = AnalyzeField(field); - if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) { - if (analysis.presence >= PDProtoScale::kLikely) { - if (cpp::CanStringBeInlined(field)) { - return PDProtoOptimization::kInline; - } + if (IsFieldInlined(field, options_)) { + return PDProtoOptimization::kInline; + } + if (IsLazy(field, options_, scc_analyzer_.get())) { + if (IsLazilyVerifiedLazy(field, options_)) { + return PDProtoOptimization::kUnverifiedLazy; } + return PDProtoOptimization::kLazy; } - if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) { - if (analysis.presence > PDProtoScale::kRarely) { - // Exclude 'never' as that may simply mean we have no data. - if (analysis.usage == PDProtoScale::kRarely) { - if (!field->is_repeated()) { - return PDProtoOptimization::kLazy; - } - } - } + if (cpp::ShouldSplit(field, options_)) { + return PDProtoOptimization::kSplit; } + return PDProtoOptimization::kNone; } @@ -156,7 +165,11 @@ class PDProtoAnalyzer { info_map_.IsCold(field, AccessInfoMap::kWrite, kColdRatio); } + cpp::Options options_; AccessInfoMap info_map_; + SplitMap split_map_; + std::unique_ptr scc_analyzer_; + const FileDescriptor* current_file_ = nullptr; }; size_t GetLongestName(const DescriptorPool& pool, absl::string_view name, @@ -291,6 +304,7 @@ absl::Status AnalyzeProfileProtoToText( if (RE2::PartialMatch(message->name(), regex)) { if (const Descriptor* descriptor = FindMessageTypeByCppName(pool, message->name())) { + analyzer.SetFile(descriptor->file()); if (analyzer.HasProfile(descriptor)) { bool message_header = false; for (int i = 0; i < descriptor->field_count(); ++i) { @@ -301,7 +315,10 @@ absl::Status AnalyzeProfileProtoToText( optimized != PDProtoOptimization::kNone) { if (!message_header) { message_header = true; - stream << "Message " << descriptor->full_name() << "\n"; + stream << "Message " + << absl::StrReplaceAll(descriptor->full_name(), + {{".", "::"}}) + << "\n"; } stream << " " << TypeName(field) << " " << field->name() << ":"; diff --git a/src/google/protobuf/compiler/cpp/tools/analyze_profile_proto_test.cc b/src/google/protobuf/compiler/cpp/tools/analyze_profile_proto_test.cc index 8b13cf06a4..04ebbf1c51 100644 --- a/src/google/protobuf/compiler/cpp/tools/analyze_profile_proto_test.cc +++ b/src/google/protobuf/compiler/cpp/tools/analyze_profile_proto_test.cc @@ -67,7 +67,7 @@ TEST(AnalyzeProfileProtoTest, LikelyStringPresence) { options.print_unused_threshold = false; options.pool = DescriptorPool::generated_pool(); EXPECT_STREQ(AnalyzeToText(info, options).c_str(), - "Message google.protobuf.compiler.tools.AnalyzeThis\n" + "Message google::protobuf::compiler::tools::AnalyzeThis\n" " string optional_string: INLINE\n"); } @@ -79,7 +79,7 @@ TEST(AnalyzeProfileProtoTest, ChildLikelyPresentAndUsed) { message { name: "google::protobuf::compiler::tools::AnalyzeThis" count: 100 - field { name: "id" getters_count: 0 configs_count: 100 } + field { name: "id" getters_count: 1000 configs_count: 100 } field { name: "optional_string" getters_count: 100 configs_count: 100 } field { name: "optional_child" getters_count: 100 configs_count: 102 } field { name: "repeated_string" getters_count: 100 configs_count: 1 } @@ -91,7 +91,7 @@ TEST(AnalyzeProfileProtoTest, ChildLikelyPresentAndUsed) { options.print_unused_threshold = false; options.pool = DescriptorPool::generated_pool(); EXPECT_STREQ(AnalyzeToText(info, options).c_str(), - "Message google.protobuf.compiler.tools.AnalyzeThis\n" + "Message google::protobuf::compiler::tools::AnalyzeThis\n" " string optional_string: INLINE\n"); } @@ -103,20 +103,19 @@ TEST(AnalyzeProfileProtoTest, ChildLikelyPresentAndRarelyUsed) { message { name: "google::protobuf::compiler::tools::AnalyzeThis" count: 100 - field { name: "id" getters_count: 0 configs_count: 100 } - field { name: "optional_string" getters_count: 100 configs_count: 100 } + field { name: "id" getters_count: 1 configs_count: 100 } + field { name: "optional_string" getters_count: 1 configs_count: 100 } field { name: "optional_child" getters_count: 100 configs_count: 1 } - field { name: "repeated_string" getters_count: 100 configs_count: 1 } - field { name: "repeated_child" getters_count: 100 configs_count: 1 } - field { name: "nested" getters_count: 0 configs_count: 1 } + field { name: "repeated_string" getters_count: 100 configs_count: 100 } + field { name: "repeated_child" getters_count: 100 configs_count: 100 } + field { name: "nested" getters_count: 1 configs_count: 100 } } )pb"); AnalyzeProfileProtoOptions options; options.print_unused_threshold = false; options.pool = DescriptorPool::generated_pool(); EXPECT_STREQ(AnalyzeToText(info, options).c_str(), - "Message google.protobuf.compiler.tools.AnalyzeThis\n" - " string optional_string: INLINE\n" + "Message google::protobuf::compiler::tools::AnalyzeThis\n" " AnalyzeChild optional_child: LAZY\n"); } @@ -134,7 +133,7 @@ TEST(AnalyzeProfileProtoTest, NestedCppNameMatchedToPoolName) { options.print_unused_threshold = false; options.pool = DescriptorPool::generated_pool(); EXPECT_STREQ(AnalyzeToText(info, options).c_str(), - "Message google.protobuf.compiler.tools.AnalyzeThis.Nested\n" + "Message google::protobuf::compiler::tools::AnalyzeThis::Nested\n" " string optional_string: INLINE\n"); } diff --git a/src/google/protobuf/compiler/cpp/tools/analyze_profile_proto_test.proto b/src/google/protobuf/compiler/cpp/tools/analyze_profile_proto_test.proto index d689886b31..74fe17ca4d 100644 --- a/src/google/protobuf/compiler/cpp/tools/analyze_profile_proto_test.proto +++ b/src/google/protobuf/compiler/cpp/tools/analyze_profile_proto_test.proto @@ -2,8 +2,10 @@ syntax = "proto2"; package google.protobuf.compiler.tools; +// Non trivial child message message AnalyzeChild { optional int32 child_id = 1; + optional AnalyzeChild child = 2; } message AnalyzeThis {