Internal change

PiperOrigin-RevId: 561756355
protocolbuffers · Sep 12, 2023 · 3ac3206 · 3ac3206
1 parent cc84eba
commit 3ac3206
Show file tree

Hide file tree

Showing 7 changed files with 109 additions and 19 deletions.
diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml
@@ -131,6 +131,7 @@ jobs:
         uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
         with:
           ref: ${{ inputs.safe-checkout }}
+          submodules: recursive
 
       - name: Setup ccache
         uses: protocolbuffers/protobuf-ci/ccache@v2
@@ -144,7 +145,7 @@ jobs:
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           command: >-
             /test.sh ${{ matrix.flags}} ${{ env.CCACHE_CMAKE_FLAGS }}
-            -Dprotobuf_BUILD_TESTS=ON -Dprotobuf_USE_EXTERNAL_GTEST=ON
+            -Dprotobuf_BUILD_TESTS=ON -Dprotobuf_USE_EXTERNAL_GTEST=ON -D CMAKE_FIND_DEBUG_MODE=ON
             -Dprotobuf_ABSL_PROVIDER=package
 
   linux-cmake-install:
@@ -155,7 +156,7 @@ jobs:
         uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
         with:
           ref: ${{ inputs.safe-checkout }}
-          submodules: recursive
+          submodules: recursive #already recursive??
 
       - name: Setup ccache
         uses: protocolbuffers/protobuf-ci/ccache@v2
@@ -165,7 +166,8 @@ jobs:
       - name: Run tests
         uses: protocolbuffers/protobuf-ci/docker@v2
         with:
-          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.10.3-1da1e086a7d1863b8bdd181ef6388a02dcd62f3a
+          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.13.3-e6272cdfe97c6df307e17b83f3a7a70844f6fc08
+          # image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.10.3-1da1e086a7d1863b8bdd181ef6388a02dcd62f3a
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           command: >-
             /install.sh -DCMAKE_CXX_STANDARD=14 ${{ env.CCACHE_CMAKE_FLAGS }} -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package \&\&
@@ -185,6 +187,7 @@ jobs:
         uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
         with:
           ref: ${{ inputs.safe-checkout }}
+          submodules: recursive
 
       - name: Setup ccache
         uses: protocolbuffers/protobuf-ci/ccache@v2
@@ -194,11 +197,12 @@ jobs:
       - name: Run tests
         uses: protocolbuffers/protobuf-ci/docker@v2
         with:
-          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.10.3-1da1e086a7d1863b8bdd181ef6388a02dcd62f3a
+          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.13.3-e6272cdfe97c6df307e17b83f3a7a70844f6fc08
+          # image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.10.3-1da1e086a7d1863b8bdd181ef6388a02dcd62f3a
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           command: >-
             /install.sh -DCMAKE_CXX_STANDARD=14 ${{ env.CCACHE_CMAKE_FLAGS }}
-            -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package
+            -Dprotobuf_USE_EXTERNAL_GTEST=ON -Dprotobuf_ABSL_PROVIDER=package -D CMAKE_FIND_DEBUG_MODE=ON
             -Dprotobuf_BUILD_EXAMPLES=OFF \&\&
             mkdir examples/build \&\&
             cd examples/build \&\&
@@ -264,7 +268,8 @@ jobs:
       - name: Run tests
         uses: protocolbuffers/protobuf-ci/docker@v2
         with:
-          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.13.3-1da1e086a7d1863b8bdd181ef6388a02dcd62f3a
+          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.13.3-e6272cdfe97c6df307e17b83f3a7a70844f6fc08
+          # image: us-docker.pkg.dev/protobuf-build/containers/test/linux/cmake:3.13.3-1da1e086a7d1863b8bdd181ef6388a02dcd62f3a
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           command: >-
             /test.sh ${{ env.CCACHE_CMAKE_FLAGS }}

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,6 +1,6 @@
 # Minimum CMake required. If available, accept the policy-controlled behavior up
 # to 3.26.
-cmake_minimum_required(VERSION 3.10...3.26)
+cmake_minimum_required(VERSION 3.11...3.26)
 
 # Revert to old behavior for MSVC debug symbols.
 if(POLICY CMP0141)

diff --git a/cmake/abseil-cpp.cmake b/cmake/abseil-cpp.cmake
@@ -1,3 +1,5 @@
+include(FetchContent)
+
 # Setup our dependency on Abseil.
 
 if(protobuf_BUILD_TESTS)
@@ -33,7 +35,17 @@ elseif(protobuf_ABSL_PROVIDER STREQUAL "module")
   endif()
 elseif(protobuf_ABSL_PROVIDER STREQUAL "package")
   # Use "CONFIG" as there is no built-in cmake module for absl.
-  find_package(absl REQUIRED CONFIG)
+  FetchContent_Declare(
+    absl
+    GIT_REPOSITORY "https://github.com/abseil/abseil-cpp.git"
+    GIT_TAG 20230802.0
+    OVERRIDE_FIND_PACKAGE)
+  FetchContent_GetProperties(absl)
+  if(NOT absl_POPULATED)
+    FetchContent_Populate(absl)
+    add_subdirectory(${absl_SOURCE_DIR} ${absl_BINARY_DIR})
+  endif()
+  find_package(absl 20230802 REQUIRED CONFIG)
 endif()
 set(_protobuf_FIND_ABSL "if(NOT TARGET absl::strings)\n  find_package(absl CONFIG)\nendif()")
 
@@ -79,6 +91,7 @@ else()
     absl::node_hash_map
     absl::node_hash_set
     absl::optional
+    absl::prefetch
     absl::span
     absl::status
     absl::statusor

diff --git a/protobuf_deps.bzl b/protobuf_deps.bzl
@@ -149,7 +149,7 @@ def protobuf_deps():
     if not native.existing_rule("upb"):
         http_archive(
             name = "upb",
-            url = "https://github.com/protocolbuffers/protobuf/archive/f85a338d79f05938d1725fba3b2c603a8d06462e.zip",
-            strip_prefix = "protobuf-f85a338d79f05938d1725fba3b2c603a8d06462e/upb",
-            sha256 = "cd28ae63e40a146ec1a2d41e96f53e637aaa5d6c746e7120d013aafc65092882",
+            url = "https://github.com/protocolbuffers/protobuf/archive/7242c3619c6db9843614b2c865681bf397261be8.zip",
+            strip_prefix = "protobuf-7242c3619c6db9843614b2c865681bf397261be8/upb",
+            sha256 = "0fc581f5e5caaf30c7119a73f2cff5d45424e4a4f23a52ebba73e3df031ad1c6",
         )
diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel
@@ -331,6 +331,7 @@ cc_library(
         ":arena_cleanup",
         ":string_block",
         "//src/google/protobuf/stubs:lite",
+        "@com_google_absl//absl/base:prefetch",
         "@com_google_absl//absl/container:layout",
         "@com_google_absl//absl/log:absl_check",
         "@com_google_absl//absl/log:absl_log",

diff --git a/src/google/protobuf/arena.cc b/src/google/protobuf/arena.cc
@@ -114,6 +114,9 @@ class GetDeallocator {
 SerialArena::SerialArena(ArenaBlock* b, ThreadSafeArena& parent)
     : ptr_{b->Pointer(kBlockHeaderSize + ThreadSafeArena::kSerialArenaSize)},
       limit_{b->Limit()},
+      prefetch_ptr_(
+          b->Pointer(kBlockHeaderSize + ThreadSafeArena::kSerialArenaSize)),
+      prefetch_limit_(b->Limit()),
       head_{b},
       space_allocated_{b->size},
       parent_{parent} {
@@ -130,9 +133,12 @@ SerialArena::SerialArena(FirstSerialArena, ArenaBlock* b,
                          ThreadSafeArena& parent)
     : head_{b}, space_allocated_{b->size}, parent_{parent} {
   if (b->IsSentry()) return;
-
-  set_ptr(b->Pointer(kBlockHeaderSize));
-  limit_ = b->Limit();
+  char* ptr = b->Pointer(kBlockHeaderSize);
+  char* limit = b->Limit();
+  set_ptr(ptr);
+  limit_ = limit;
+  prefetch_ptr_ = ptr;
+  prefetch_limit_ = limit;
 }
 
 std::vector<void*> SerialArena::PeekCleanupListForTesting() {
@@ -159,8 +165,12 @@ std::vector<void*> ThreadSafeArena::PeekCleanupListForTesting() {
 }
 
 void SerialArena::Init(ArenaBlock* b, size_t offset) {
-  set_ptr(b->Pointer(offset));
-  limit_ = b->Limit();
+  char* ptr = b->Pointer(offset);
+  char* limit = b->Limit();
+  set_ptr(ptr);
+  limit_ = limit;
+  prefetch_ptr_ = ptr;
+  prefetch_limit_ = limit;
   head_.store(b, std::memory_order_relaxed);
   space_used_.store(0, std::memory_order_relaxed);
   space_allocated_.store(b->size, std::memory_order_relaxed);
@@ -268,8 +278,12 @@ void SerialArena::AllocateNewBlock(size_t n) {
                                             /*used=*/used,
                                             /*allocated=*/mem.n, wasted);
   auto* new_head = new (mem.p) ArenaBlock{old_head, mem.n};
-  set_ptr(new_head->Pointer(kBlockHeaderSize));
-  limit_ = new_head->Limit();
+  char* new_ptr = new_head->Pointer(kBlockHeaderSize);
+  char* new_limit = new_head->Limit();
+  set_ptr(new_ptr);
+  limit_ = new_limit;
+  prefetch_ptr_ = new_ptr;
+  prefetch_limit_ = new_limit;
   // Previous writes must take effect before writing new head.
   head_.store(new_head, std::memory_order_release);
 

diff --git a/src/google/protobuf/serial_arena.h b/src/google/protobuf/serial_arena.h
@@ -12,6 +12,7 @@
 
 #include <algorithm>
 #include <atomic>
+#include <cstddef>
 #include <cstdint>
 #include <string>
 #include <type_traits>
@@ -21,6 +22,8 @@
 
 #include "google/protobuf/stubs/common.h"
 #include "absl/base/attributes.h"
+#include "absl/base/optimization.h"
+#include "absl/base/prefetch.h"
 #include "absl/log/absl_check.h"
 #include "absl/numeric/bits.h"
 #include "google/protobuf/arena_align.h"
@@ -29,7 +32,6 @@
 #include "google/protobuf/port.h"
 #include "google/protobuf/string_block.h"
 
-
 // Must be included last.
 #include "google/protobuf/port_def.inc"
 
@@ -225,6 +227,7 @@ class PROTOBUF_EXPORT SerialArena {
     PROTOBUF_UNPOISON_MEMORY_REGION(ret, n);
     *out = ret;
     set_ptr(reinterpret_cast<char*>(next));
+    MaybePrefetchForwards(reinterpret_cast<char*>(next));
     return true;
   }
 
@@ -251,6 +254,7 @@ class PROTOBUF_EXPORT SerialArena {
     set_ptr(reinterpret_cast<char*>(next));
     AddCleanupFromExisting(ret, destructor);
     ABSL_DCHECK_GE(limit_, ptr());
+    MaybePrefetchForwards(reinterpret_cast<char*>(next));
     return ret;
   }
 
@@ -279,10 +283,58 @@ class PROTOBUF_EXPORT SerialArena {
 
     PROTOBUF_UNPOISON_MEMORY_REGION(limit_ - n, n);
     limit_ -= n;
+    MaybePrefetchBackwards(limit_);
     ABSL_DCHECK_GE(limit_, ptr());
     cleanup::CreateNode(tag, limit_, elem, destructor);
   }
 
+  static constexpr ptrdiff_t kPrefetchForwardsDegree = ABSL_CACHELINE_SIZE * 16;
+  static constexpr ptrdiff_t kPrefetchBackwardsDegree = ABSL_CACHELINE_SIZE * 6;
+
+  // Prefetch the next kPrefetchForwardsDegree bytes after `prefetch_ptr_` and
+  // up to `prefetch_limit_`, if `next` is within kPrefetchForwardsDegree bytes
+  // of `prefetch_ptr_`.
+  PROTOBUF_ALWAYS_INLINE
+  void MaybePrefetchForwards(const char* next) {
+    ABSL_DCHECK(static_cast<const void*>(prefetch_ptr_) == nullptr ||
+                static_cast<const void*>(prefetch_ptr_) >= head());
+    if (PROTOBUF_PREDICT_TRUE(prefetch_ptr_ - next > kPrefetchForwardsDegree))
+      return;
+    if (PROTOBUF_PREDICT_TRUE(prefetch_ptr_ < prefetch_limit_)) {
+      const char* prefetch_ptr = std::max(next, prefetch_ptr_);
+      ABSL_DCHECK(prefetch_ptr != nullptr);
+      const char* end =
+          std::min(prefetch_limit_, prefetch_ptr + ABSL_CACHELINE_SIZE * 16);
+      for (; prefetch_ptr < end; prefetch_ptr += ABSL_CACHELINE_SIZE) {
+        absl::PrefetchToLocalCacheForWrite(prefetch_ptr);
+      }
+      prefetch_ptr_ = prefetch_ptr;
+    }
+  }
+
+  PROTOBUF_ALWAYS_INLINE
+  // Prefetch up to kPrefetchBackwardsDegree before `prefetch_limit_` and after
+  // `prefetch_ptr_`, if `limit` is within  kPrefetchBackwardsDegree of
+  // `prefetch_limit_`.
+  void MaybePrefetchBackwards(const char* limit) {
+    ABSL_DCHECK(prefetch_limit_ == nullptr ||
+                static_cast<const void*>(prefetch_limit_) <=
+                    static_cast<const void*>(head()->Limit()));
+    if (PROTOBUF_PREDICT_TRUE(limit - prefetch_limit_ >
+                              kPrefetchBackwardsDegree))
+      return;
+    if (PROTOBUF_PREDICT_TRUE(prefetch_limit_ > prefetch_ptr_)) {
+      const char* prefetch_limit = std::min(limit, prefetch_limit_);
+      ABSL_DCHECK_NE(prefetch_limit, nullptr);
+      const char* end =
+          std::max(prefetch_ptr_, prefetch_limit - kPrefetchBackwardsDegree);
+      for (; prefetch_limit > end; prefetch_limit -= ABSL_CACHELINE_SIZE) {
+        absl::PrefetchToLocalCacheForWrite(prefetch_limit);
+      }
+      prefetch_limit_ = prefetch_limit;
+    }
+  }
+
  private:
   friend class ThreadSafeArena;
 
@@ -319,6 +371,11 @@ class PROTOBUF_EXPORT SerialArena {
   std::atomic<char*> ptr_{nullptr};
   // Limiting address up to which memory can be allocated from the head block.
   char* limit_ = nullptr;
+  // Current prefetch positions. Data from `ptr_` up to but not including
+  // `prefetch_ptr_` is software prefetched. Similarly, data from `limit_` down
+  // to but not including `prefetch_limit_` is software prefetched.
+  const char* prefetch_ptr_ = nullptr;
+  const char* prefetch_limit_ = nullptr;
 
   // The active string block.
   std::atomic<StringBlock*> string_block_{nullptr};