Automated rollback of commit 2b600ddf24.

PiperOrigin-RevId: 498032941

src/google/protobuf/BUILD.bazel

@@ -332,6 +332,7 @@ cc_library(
         ":arena_config",
         "//src/google/protobuf/io",
         "//src/google/protobuf/stubs:lite",
+        "@com_google_absl//absl/container:btree",
         "@com_google_absl//absl/container:flat_hash_set",
         "@com_google_absl//absl/numeric:bits",
         "@com_google_absl//absl/strings:internal",

src/google/protobuf/compiler/parser_unittest.cc

@@ -35,8 +35,9 @@
 #include "google/protobuf/compiler/parser.h"
 
 #include <algorithm>
-#include <map>
 #include <memory>
+#include <string>
+#include <utility>
 #include <vector>
 
 #include "google/protobuf/any.pb.h"
@@ -46,6 +47,7 @@
 #include <gtest/gtest.h>
 #include "absl/container/flat_hash_map.h"
 #include "google/protobuf/stubs/logging.h"
+#include "absl/memory/memory.h"
 #include "absl/strings/str_join.h"
 #include "absl/strings/substitute.h"
 #include "google/protobuf/test_util2.h"
@@ -85,7 +87,7 @@ class MockValidationErrorCollector : public DescriptorPool::ErrorCollector {
                                io::ErrorCollector* wrapped_collector)
       : source_locations_(source_locations),
         wrapped_collector_(wrapped_collector) {}
-  ~MockValidationErrorCollector() override {}
+  ~MockValidationErrorCollector() override = default;
 
   // implements ErrorCollector ---------------------------------------
   void AddError(const std::string& filename, const std::string& element_name,
@@ -111,9 +113,10 @@ class ParserTest : public testing::Test {
 
   // Set up the parser to parse the given text.
   void SetupParser(const char* text) {
-    raw_input_.reset(new io::ArrayInputStream(text, strlen(text)));
-    input_.reset(new io::Tokenizer(raw_input_.get(), &error_collector_));
-    parser_.reset(new Parser());
+    raw_input_ = absl::make_unique<io::ArrayInputStream>(text, strlen(text));
+    input_ =
+        absl::make_unique<io::Tokenizer>(raw_input_.get(), &error_collector_);
+    parser_ = absl::make_unique<Parser>();
     parser_->RecordErrorsTo(&error_collector_);
     parser_->SetRequireSyntaxIdentifier(require_syntax_identifier_);
   }
@@ -2798,16 +2801,18 @@ class SourceInfoTest : public ParserTest {
         return false;
       }
 
-      spans_.insert(
-          std::make_pair(SpanKey(*descriptor_proto, field, index), &location));
+      spans_[SpanKey(*descriptor_proto, field, index)].push_back(&location);
     }
 
     return true;
   }
 
   void TearDown() override {
-    EXPECT_TRUE(spans_.empty()) << "Forgot to call HasSpan() for:\n"
-                                << spans_.begin()->second->DebugString();
+    for (auto& kv : spans_) {
+      EXPECT_TRUE(kv.second.empty())
+          << "Forgot to call HasSpan() for "
+          << (*kv.second.begin())->DebugString() << " spans.";
+    }
   }
 
   // -----------------------------------------------------------------
@@ -2879,59 +2884,52 @@ class SourceInfoTest : public ParserTest {
                           const char* expected_leading_comments,
                           const char* expected_trailing_comments,
                           const char* expected_leading_detached_comments) {
-    std::pair<SpanMap::iterator, SpanMap::iterator> range =
-        spans_.equal_range(SpanKey(descriptor_proto, field, index));
-
+    SpanKey key(descriptor_proto, field, index);
     if (start_marker == '\0') {
-      if (range.first == range.second) {
-        return false;
-      } else {
-        spans_.erase(range.first);
-        return true;
-      }
-    } else {
-      std::pair<int, int> start_pos = markers_.at(start_marker);
-      std::pair<int, int> end_pos = markers_.at(end_marker);
-
-      RepeatedField<int> expected_span;
-      expected_span.Add(start_pos.first);
-      expected_span.Add(start_pos.second);
-      if (end_pos.first != start_pos.first) {
-        expected_span.Add(end_pos.first);
-      }
-      expected_span.Add(end_pos.second);
-
-      for (SpanMap::iterator iter = range.first; iter != range.second; ++iter) {
-        if (CompareSpans(expected_span, iter->second->span())) {
-          if (expected_leading_comments == nullptr) {
-            EXPECT_FALSE(iter->second->has_leading_comments());
-          } else {
-            EXPECT_TRUE(iter->second->has_leading_comments());
-            EXPECT_EQ(expected_leading_comments,
-                      iter->second->leading_comments());
-          }
-          if (expected_trailing_comments == nullptr) {
-            EXPECT_FALSE(iter->second->has_trailing_comments());
-          } else {
-            EXPECT_TRUE(iter->second->has_trailing_comments());
-            EXPECT_EQ(expected_trailing_comments,
-                      iter->second->trailing_comments());
-          }
-          if (expected_leading_detached_comments == nullptr) {
-            EXPECT_EQ(0, iter->second->leading_detached_comments_size());
-          } else {
-            EXPECT_EQ(
-                expected_leading_detached_comments,
-                absl::StrJoin(iter->second->leading_detached_comments(), "\n"));
-          }
-
-          spans_.erase(iter);
-          return true;
+      auto old = spans_.extract(key);
+      // Return true if we actually removed something.
+      return !(old.empty() || old.mapped().empty());
+    }
+
+    std::vector<const SourceCodeInfo::Location*>& range = spans_[key];
+    std::pair<int, int> start_pos = markers_.at(start_marker);
+    std::pair<int, int> end_pos = markers_.at(end_marker);
+
+    RepeatedField<int> expected_span;
+    expected_span.Add(start_pos.first);
+    expected_span.Add(start_pos.second);
+    if (end_pos.first != start_pos.first) {
+      expected_span.Add(end_pos.first);
+    }
+    expected_span.Add(end_pos.second);
+
+    for (auto iter = range.begin(); iter != range.end(); ++iter) {
+      const SourceCodeInfo::Location* location = *iter;
+      if (CompareSpans(expected_span, location->span())) {
+        if (expected_leading_comments == nullptr) {
+          EXPECT_FALSE(location->has_leading_comments());
+        } else {
+          EXPECT_TRUE(location->has_leading_comments());
+          EXPECT_EQ(expected_leading_comments, location->leading_comments());
+        }
+        if (expected_trailing_comments == nullptr) {
+          EXPECT_FALSE(location->has_trailing_comments());
+        } else {
+          EXPECT_TRUE(location->has_trailing_comments());
+          EXPECT_EQ(expected_trailing_comments, location->trailing_comments());
+        }
+        if (expected_leading_detached_comments == nullptr) {
+          EXPECT_EQ(0, location->leading_detached_comments_size());
+        } else {
+          EXPECT_EQ(expected_leading_detached_comments,
+                    absl::StrJoin(location->leading_detached_comments(), "\n"));
         }
+        range.erase(iter);
+        return true;
       }
+    }
 
       return false;
-    }
   }
 
  private:
@@ -2940,24 +2938,28 @@ class SourceInfoTest : public ParserTest {
     const FieldDescriptor* field;
     int index;
 
-    inline SpanKey() {}
+    inline SpanKey() = default;
     inline SpanKey(const Message& descriptor_proto_param,
                    const FieldDescriptor* field_param, int index_param)
         : descriptor_proto(&descriptor_proto_param),
           field(field_param),
           index(index_param) {}
 
-    inline bool operator<(const SpanKey& other) const {
-      if (descriptor_proto < other.descriptor_proto) return true;
-      if (descriptor_proto > other.descriptor_proto) return false;
-      if (field < other.field) return true;
-      if (field > other.field) return false;
-      return index < other.index;
+    template <typename H>
+    friend H AbslHashValue(H h, const SpanKey& key) {
+      return H::combine(std::move(h), key.descriptor_proto, key.field,
+                        key.index);
+    }
+
+    friend bool operator==(const SpanKey& lhs, const SpanKey& rhs) {
+      return lhs.descriptor_proto == rhs.descriptor_proto &&  //
+             lhs.field == rhs.field &&                        //
+             lhs.index == rhs.index;
     }
   };
 
-  typedef std::multimap<SpanKey, const SourceCodeInfo::Location*> SpanMap;
-  SpanMap spans_;
+  absl::flat_hash_map<SpanKey, std::vector<const SourceCodeInfo::Location*>>
+      spans_;
   absl::flat_hash_map<char, std::pair<int, int>> markers_;
   std::string text_without_markers_;
 

src/google/protobuf/descriptor.cc

@@ -39,7 +39,6 @@
 #include <cstdlib>
 #include <functional>
 #include <limits>
-#include <map>
 #include <memory>
 #include <sstream>
 #include <string>

src/google/protobuf/descriptor_database.h

@@ -37,7 +37,6 @@
 #ifndef GOOGLE_PROTOBUF_DESCRIPTOR_DATABASE_H__
 #define GOOGLE_PROTOBUF_DESCRIPTOR_DATABASE_H__
 
-#include <map>
 #include <string>
 #include <utility>
 #include <vector>

src/google/protobuf/extension_set.h

@@ -40,12 +40,12 @@
 
 #include <algorithm>
 #include <cassert>
-#include <map>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "google/protobuf/stubs/common.h"
+#include "absl/container/btree_map.h"
 #include "google/protobuf/stubs/logging.h"
 #include "google/protobuf/port.h"
 #include "google/protobuf/port.h"
@@ -680,7 +680,7 @@ class PROTOBUF_EXPORT ExtensionSet {
     };
   };
 
-  typedef std::map<int, Extension> LargeMap;
+  using LargeMap = absl::btree_map<int, Extension>;
 
   // Wrapper API that switches between flat-map and LargeMap.
 

src/google/protobuf/io/printer.cc

@@ -38,7 +38,6 @@
 
 #include <cstddef>
 #include <functional>
-#include <map>
 #include <string>
 #include <type_traits>
 #include <utility>

src/google/protobuf/map.h

@@ -42,7 +42,6 @@
 #include <initializer_list>
 #include <iterator>
 #include <limits>  // To support Visual Studio 2008
-#include <map>
 #include <string>
 #include <type_traits>
 #include <utility>
@@ -57,6 +56,7 @@
 
 #include "google/protobuf/stubs/common.h"
 #include "google/protobuf/arena.h"
+#include "absl/container/btree_map.h"
 #include "google/protobuf/generated_enum_util.h"
 #include "google/protobuf/map_type_handler.h"
 #include "google/protobuf/port.h"
@@ -305,8 +305,8 @@ using LessForTree = typename TransparentSupport<
 
 template <typename Key>
 using TreeForMap =
-    std::map<KeyForTree<Key>, NodeBase*, LessForTree<Key>,
-             MapAllocator<std::pair<const KeyForTree<Key>, NodeBase*>>>;
+    absl::btree_map<KeyForTree<Key>, NodeBase*, LessForTree<Key>,
+                    MapAllocator<std::pair<const KeyForTree<Key>, NodeBase*>>>;
 
 // Type safe tagged pointer.
 // We convert to/from nodes and trees using the operations below.

src/google/protobuf/map_field_test.cc

@@ -28,7 +28,6 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-#include <map>
 #include <memory>
 
 #include "google/protobuf/arena.h"

src/google/protobuf/map_test.inc

@@ -381,56 +381,6 @@ static int k0 = 812398771;
 static int k1 = 1312938717;
 static int k2 = 1321555333;
 
-// A naive begin() implementation will cause begin() to get slower and slower
-// if one erases elements at the "front" of the hash map, and we'd like to
-// avoid that, as std::unordered_map does.
-TEST_F(MapImplTest, BeginIsFast) {
-  if (true) return;  // TODO(gpike): make this less flaky and re-enable it.
-  Map<int32_t, int32_t> map;
-  const int kTestSize = 250000;
-  // Create a random-looking map of size n.  Use non-negative integer keys.
-  uint32_t frog = 123983;
-  int last_key = 0;
-  int counter = 0;
-  while (map.size() < kTestSize) {
-    frog *= static_cast<uint32_t>(k0);
-    frog ^= frog >> 17;
-    frog += counter++;
-    last_key =
-        static_cast<int>(frog) >= 0 ? static_cast<int>(frog) : last_key ^ 1;
-    GOOGLE_ABSL_DCHECK_GE(last_key, 0);
-    map[last_key] = last_key ^ 1;
-  }
-  std::vector<int64_t> times;
-  // We're going to do map.erase(map.begin()) over and over again.  But,
-  // just in case one iteration is fast compared to the granularity of
-  // our time keeping, we measure kChunkSize iterations per outer-loop iter.
-  const int kChunkSize = 1000;
-  GOOGLE_ABSL_CHECK_EQ(kTestSize % kChunkSize, 0);
-  do {
-    const int64_t start = Now();
-    for (int i = 0; i < kChunkSize; i++) {
-      map.erase(map.begin());
-    }
-    const int64_t end = Now();
-    if (end > start) {
-      times.push_back(end - start);
-    }
-  } while (!map.empty());
-  if (times.size() < .99 * kTestSize / kChunkSize) {
-    GOOGLE_ABSL_LOG(WARNING) << "Now() isn't helping us measure time";
-    return;
-  }
-  int64_t x0 = median(times.begin(), times.begin() + 9);
-  int64_t x1 = median(times.begin() + times.size() - 9, times.end());
-  GOOGLE_ABSL_LOG(INFO) << "x0=" << x0 << ", x1=" << x1;
-  // x1 will greatly exceed x0 if the code we just executed took O(n^2) time.
-  // And we'll probably time out and never get here.  So, this test is
-  // intentionally loose: we check that x0 and x1 are within a factor of 8.
-  EXPECT_GE(x1, x0 / 8);
-  EXPECT_GE(x0, x1 / 8);
-}
-
 // Try to create kTestSize keys that will land in just a few buckets, and
 // time the insertions, to get a rough estimate of whether an O(n^2) worst case
 // was triggered.  This test is a hacky, but probably better than nothing.

src/google/protobuf/stubs/common.h

@@ -37,7 +37,6 @@
 
 #include <algorithm>
 #include <iostream>
-#include <map>
 #include <memory>
 #include <string>
 #include <vector>