Merge pull request #1521 from tbkka/tbkka-performance-harness

tbkka · web-flow · commit 4ac8a2e399f1 · 2023-12-06T15:55:12.000-08:00
Get the performance harness running again...
diff --git a/Performance/Harness.h b/Performance/Harness.h
@@ -94,7 +94,7 @@ class Harness {
    * from the main results.
    */
   template <typename Function>
-  typename std::result_of<Function()>::type measure_subtask(
+  typename std::invoke_result<Function>::type measure_subtask(
       const std::string& name, Function&& func);
 
   /**
@@ -171,7 +171,7 @@ void Harness::measure(const Function& func) {
 }
 
 template <typename Function>
-typename std::result_of<Function()>::type Harness::measure_subtask(
+typename std::invoke_result<Function>::type Harness::measure_subtask(
   const std::string& name, Function&& func) {
   subtask_names.push_back(name);
   using std::chrono::steady_clock;
diff --git a/Performance/Harness.swift b/Performance/Harness.swift
@@ -106,7 +106,7 @@ class Harness: GeneratedHarnessMembers {
 
         for name in taskNames {
           let time = currentSubtasks[name] ?? 0
-          print(String(format: "%9.3f", name, time), terminator: "")
+          print(String(format: "%9.3f", time), terminator: "")
           subtaskTimings[name] = (subtaskTimings[name] ?? []) + [time]
         }
         print()
diff --git a/Performance/generators/cpp.sh b/Performance/generators/cpp.sh
@@ -98,7 +98,6 @@ using google::protobuf::util::BinaryToJsonString;
 using google::protobuf::util::JsonToBinaryString;
 using google::protobuf::util::MessageDifferencer;
 using google::protobuf::util::NewTypeResolverForDescriptorPool;
-using google::protobuf::util::Status;
 using google::protobuf::util::TypeResolver;
 using std::cerr;
 using std::endl;
@@ -148,12 +147,14 @@ void Harness::run() {
       // Exercise JSON serialization.
       auto json = measure_subtask("Encode JSON", [&]() {
         string out_json;
-        BinaryToJsonString(type_resolver, *type_url, data, &out_json);
+        auto r = BinaryToJsonString(type_resolver, *type_url, data, &out_json);
+        (void)r; // UNUSED
         return out_json;
       });
       auto decoded_binary = measure_subtask("Decode JSON", [&]() {
         string out_binary;
-        JsonToBinaryString(type_resolver, *type_url, json, &out_binary);
+        auto r = JsonToBinaryString(type_resolver, *type_url, json, &out_binary);
+        (void)r; // UNUSED
         return out_binary;
       });
 
diff --git a/Performance/generators/swift.sh b/Performance/generators/swift.sh
@@ -106,20 +106,20 @@ extension Harness {
         populateFields(of: &message)
       }
 
-      message = measureSubtask("Populate fields with with") {
+      message = measureSubtask("Populate (with)") {
         return populateFieldsWithWith()
       }
 
       // Exercise binary serialization.
-      let data = try measureSubtask("Encode binary") {
-        return try message.serializedBytes()
+      let data = try measureSubtask("Encode binary") { () -> Data in
+        try message.serializedBytes()
       }
       let message2 = try measureSubtask("Decode binary") {
         return try PerfMessage(serializedData: data)
       }
 
       // Exercise JSON serialization.
-      let json = try measureSubtask("Encode JSON") {
+      let json = try measureSubtask("Encode JSON") { () -> Data in
         return try message.jsonUTF8Bytes()
       }
       _ = try measureSubtask("Decode JSON") {
diff --git a/Performance/perf_runner.sh b/Performance/perf_runner.sh
@@ -41,12 +41,12 @@ else
 fi
 
 # Directory containing this script
-readonly script_dir="."
+readonly script_dir=`pwd`
 
 # Change this if your checkout of github.com/protocolbuffers/protobuf is in a
 # different location.
 readonly GOOGLE_PROTOBUF_CHECKOUT=${GOOGLE_PROTOBUF_CHECKOUT:-"$script_dir/../../protobuf"}
-readonly PROTOC=${PROTOC:-"${GOOGLE_PROTOBUF_CHECKOUT}/src/protoc"}
+readonly PROTOC=${PROTOC:-"${GOOGLE_PROTOBUF_CHECKOUT}/protoc"}
 
 function usage() {
   cat >&2 <<EOF
@@ -123,7 +123,7 @@ EOF
 
   echo "Running $language test harness alone..."
   sleep 3
-  DYLD_LIBRARY_PATH="$script_dir/_generated" "$harness" "$partial_results"
+  DYLD_LIBRARY_PATH=`dirname $harness` "$harness" "$partial_results"
   sleep 3
 
   cp "$harness" "${harness}_stripped"
@@ -151,8 +151,10 @@ function profile_harness() {
   perf_dir="$3"
 
   echo "Running $description test harness in Instruments..."
-  instruments -t "$script_dir/Protobuf" -D "$results_trace" \
-      "$harness" -e DYLD_LIBRARY_PATH "$perf_dir/_generated"
+  mkdir -p "$results_trace"
+  xctrace record --template 'Time Profiler' --output "$results_trace" \
+          --env DYLD_LIBRARY_PATH="$perf_dir/_generated" \
+          --launch -- "$harness"
 }
 
 # Inserts the partial visualization results from all the languages tested into
@@ -236,10 +238,14 @@ fi
 # Set up a hook to cleanup revision comparison checkouts when the script
 # completes.
 declare -a CLEANUP_WHEN_DONE
+GIT_WORKTREE=""
 function cleanup_revision_checkouts() {
   if [[ "${#CLEANUP_WHEN_DONE[@]}" -ne 0 ]]; then
     rm -rf "${CLEANUP_WHEN_DONE[@]}"
   fi
+  if [ "$GIT_WORKTREE" != "" ]; then
+    git worktree remove "$GIT_WORKTREE"
+  fi
 }
 trap cleanup_revision_checkouts EXIT HUP INT QUIT TERM
 
@@ -297,7 +303,9 @@ for comparison in "${comparisons[@]}"; do
     echo "==== Building/running C++ harness ===================="
     echo
 
-    ${PROTOC} --cpp_out="$script_dir" "$gen_message_path"
+    ${PROTOC} --cpp_out="$script_dir/_generated" \
+              --proto_path=`dirname $gen_message_path` \
+              "$gen_message_path"
 
     harness_cpp="$script_dir/_generated/harness_cpp"
     run_cpp_harness "$harness_cpp"
@@ -314,19 +322,20 @@ for comparison in "${comparisons[@]}"; do
 
       # Check out the commit to a temporary directory and create its _generated
       # directory. (Results will still go in the working tree.)
-      tmp_checkout="$(mktemp -d -t swiftprotoperf)"
-      CLEANUP_WHEN_DONE+=("$tmp_checkout")
-      git --work-tree="$tmp_checkout" checkout "$comparison" -- .
-      mkdir "$tmp_checkout/Performance/_generated"
-
-      build_swift_packages "$tmp_checkout" "ForRev"
-      ${PROTOC} --plugin="$tmp_checkout/.build/release/protoc-gen-swiftForRev" \
-          --swiftForRev_out=FileNaming=DropPath:"$tmp_checkout/Performance/_generated" \
-          "$gen_message_path"
-
-      harness_swift="$tmp_checkout/Performance/_generated/harness_swift"
+      GIT_WORKTREE="$(mktemp -d `pwd`/_generated/swiftprotoperf.XXXXXX)"
+      CLEANUP_WHEN_DONE+=("$GIT_WORKTREE")
+      git worktree add "$GIT_WORKTREE" "$comparison"
+      mkdir "$GIT_WORKTREE/Performance/_generated"
+
+      build_swift_packages "$GIT_WORKTREE" "ForRev"
+      ${PROTOC} --plugin="$GIT_WORKTREE/.build/release/protoc-gen-swiftForRev" \
+                --swiftForRev_out=FileNaming=DropPath:"$GIT_WORKTREE/Performance/_generated" \
+                --proto_path=`dirname $gen_message_path` \
+                "$gen_message_path"
+
+      harness_swift="$GIT_WORKTREE/Performance/_generated/harness_swift"
       results_trace="$script_dir/_results/$report_type (swift)"
-      run_swift_harness "$tmp_checkout" "$comparison" "$commit_results"
+      run_swift_harness "$GIT_WORKTREE" "$comparison" "$commit_results"
     else
       echo
       echo "==== Found cached results for Swift ($comparison) ===================="
@@ -341,10 +350,11 @@ echo "==== Building/running Swift harness (working tree) ===================="
 echo
 
 build_swift_packages "$script_dir/.." "ForWorkTree"
-${PROTOC} --plugin="$script_dir/../.build/release/protoc-gen-swiftForWorkTree" \
-    --swiftForWorkTree_out=FileNaming=DropPath:"$script_dir/_generated" \
-    --cpp_out="$script_dir" \
-    "$gen_message_path"
+
+${PROTOC} --plugin="$script_dir/../.build/release/protoc-gen-swift" \
+          --swift_out=FileNaming=DropPath:`dirname $gen_message_path` \
+          --proto_path=`dirname $gen_message_path` \
+          "$gen_message_path"
 
 harness_swift="$script_dir/_generated/harness_swift"
 results_trace="$script_dir/_results/$report_type (swift)"
@@ -359,6 +369,5 @@ EOF
 
 insert_visualization_results "$partial_results" "$results_js"
 
-# Open the Instruments trace and HTML report at the end.
-open -g "$display_results_trace.trace"
+# Open the HTML report at the end.
 open -g "$script_dir/harness-visualization.html"
diff --git a/Performance/runners/CMakeLists.txt b/Performance/runners/CMakeLists.txt
@@ -0,0 +1,63 @@
+#
+# cmake configuration file for building the C++ test harness
+#
+# This assumes you have a protobuf source checkout handy,
+# and have used `git submodule update` to obtain all the
+# related source repos.
+#
+# This script uses the protobuf sources to build libprotobuf and
+# statically links it into the test harness executable.
+# (This is probably not necessary; updates to this file to
+# use a better strategy would be appreciated.)
+#
+# Also assumes that you have abseil_cpp and googletest installed
+# locally via e.g.,
+#   brew install googletest
+#   brew install abseil
+#
+
+cmake_minimum_required(VERSION 3.10...3.26)
+set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+set(CMAKE_BUILD_TYPE RelWithDebInfo)
+
+project(swiftprotobuf-perf C CXX)
+
+# Update this with the appropriate path to the protobuf source
+# checkout, starting from the directory holding this file.
+# Default here assumes that `protobuf` is checked out beside
+# `swift-protobuf`
+set(protobuf_SOURCE_DIR ${CMAKE_SOURCE_DIR}/../../../protobuf)
+set(protobuf_VERSION "999.999")
+
+# Use protobuf cmake scripts to locate suitable abseil package:
+set(protobuf_ABSL_PROVIDER "package")
+include(${protobuf_SOURCE_DIR}/cmake/abseil-cpp.cmake)
+
+# Use protobuf cmake scripts for building libprotobuf
+include(${protobuf_SOURCE_DIR}/cmake/libprotobuf.cmake)
+
+# Use utf8_range from protobuf checkout
+set(utf8_range_SOURCE_DIR ${protobuf_SOURCE_DIR}/third_party/utf8_range)
+add_subdirectory(${utf8_range_SOURCE_DIR} third_party/utf8_range)
+
+include_directories(
+  ${protobuf_SOURCE_DIR}/src
+  ${utf8_range_SOURCE_DIR}
+)
+
+add_executable(harness_cpp
+  ../main.cc
+  ../Harness.cc
+  ../_generated/Harness+Generated.cc
+  ../_generated/message.pb.cc
+)
+
+target_include_directories(harness_cpp PRIVATE
+  ${CMAKE_SOURCE_DIR}
+  ${CMAKE_SOURCE_DIR}/..
+  ${ABSL_ROOT_DIR}
+  ${utf8_range_SOURCE_DIR}
+)
+
+target_link_libraries(harness_cpp PRIVATE libprotobuf)
diff --git a/Performance/runners/cpp.sh b/Performance/runners/cpp.sh
@@ -26,25 +26,16 @@ function run_cpp_harness() {
     gen_harness_path="$script_dir/_generated/Harness+Generated.cc"
     generate_cpp_harness
 
-    echo "Building C++ test harness..."
-    time ( g++ --std=c++11 -O \
-        -o "$harness" \
-        -I "$script_dir" \
-        -I "$GOOGLE_PROTOBUF_CHECKOUT/src" \
-        -L "$GOOGLE_PROTOBUF_CHECKOUT/src/.libs" \
-        -lprotobuf \
-        "$gen_harness_path" \
-        "$script_dir/Harness.cc" \
-        "$script_dir/_generated/message.pb.cc" \
-        "$script_dir/main.cc" \
-    )
+    echo
+    echo "Building C++ libprotobuf and performance test harness..."
     echo
 
-    # Make sure the dylib is loadable from the harness if the user hasn't
-    # actually installed them.
-    cp "$GOOGLE_PROTOBUF_CHECKOUT"/src/.libs/libprotobuf.*.dylib \
-        "$script_dir/_generated"
+    pushd $script_dir/runners
+    cmake -B ../_generated -S .
+    cmake --build ../_generated
+    popd
 
     run_harness_and_concatenate_results "C++" "$harness" "$partial_results"
+    echo
   )
 }

Original file line number	Diff line number	Diff line change
`@@ -106,7 +106,7 @@ class Harness: GeneratedHarnessMembers {`
`106`	`106`
`107`	`107`	`for name in taskNames {`
`108`	`108`	`let time = currentSubtasks[name] ?? 0`
`109`		`- print(String(format: "%9.3f", name, time), terminator: "")`
	`109`	`+ print(String(format: "%9.3f", time), terminator: "")`
`110`	`110`	`subtaskTimings[name] = (subtaskTimings[name] ?? []) + [time]`
`111`	`111`	`}`
`112`	`112`	`print()`
Original file line number	Diff line number	Diff line change
`@@ -106,20 +106,20 @@ extension Harness {`
`106`	`106`	`populateFields(of: &message)`
`107`	`107`	`}`
`108`	`108`
`109`		`- message = measureSubtask("Populate fields with with") {`
	`109`	`+ message = measureSubtask("Populate (with)") {`
`110`	`110`	`return populateFieldsWithWith()`
`111`	`111`	`}`
`112`	`112`
`113`	`113`	`// Exercise binary serialization.`
`114`		`- let data = try measureSubtask("Encode binary") {`
`115`		`- return try message.serializedBytes()`
	`114`	`+ let data = try measureSubtask("Encode binary") { () -> Data in`
	`115`	`+ try message.serializedBytes()`
`116`	`116`	`}`
`117`	`117`	`let message2 = try measureSubtask("Decode binary") {`
`118`	`118`	`return try PerfMessage(serializedData: data)`
`119`	`119`	`}`
`120`	`120`
`121`	`121`	`// Exercise JSON serialization.`
`122`		`- let json = try measureSubtask("Encode JSON") {`
	`122`	`+ let json = try measureSubtask("Encode JSON") { () -> Data in`
`123`	`123`	`return try message.jsonUTF8Bytes()`
`124`	`124`	`}`
`125`	`125`	`_ = try measureSubtask("Decode JSON") {`