bryancall commented on code in PR #13670:
URL: https://github.com/apache/trafficserver/pull/13670#discussion_r3997342748


##########
tools/benchmark/benchmark_Regex.cc:
##########
@@ -0,0 +1,512 @@
+/** @file
+
+  Benchmarks for the tsutil Regex wrapper: time per operation and the number 
and size
+  of heap allocations each operation makes.
+
+  The allocation half matters as much as the timing half. PCRE2 routes every 
allocation
+  it makes for a compile or a match through the callbacks the wrapper 
installs, and those
+  call the system allocator, so counting calls to malloc across a region 
counts exactly
+  what the wrapper caused. Under the just-in-time engine a match should reach 
the system
+  allocator zero times; the interpreter allocates a frames vector and does not.
+
+  Interposing malloc is only wired up on Linux, where defining these symbols 
in the
+  executable is enough. Elsewhere the counters stay at zero and the report 
says so, so a
+  run on another platform still gives timings without quietly reporting zero 
allocations
+  as a result.
+
+  @section license License
+
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <string>
+#include <string_view>
+#include <vector>
+
+#define CATCH_CONFIG_ENABLE_BENCHMARKING
+#include <catch2/catch_test_macros.hpp>
+#include <catch2/benchmark/catch_benchmark.hpp>
+
+#include "tsutil/Regex.h"
+
+// ---------------------------------------------------------------------------
+// Allocation counting
+// ---------------------------------------------------------------------------
+
+namespace
+{
+struct AllocStats {
+  unsigned long calls = 0;
+  unsigned long bytes = 0;
+};
+
+// Counting is per thread so a benchmark that spawns threads does not race the 
counters.
+// These benchmarks are single threaded; the qualifier is here so the numbers 
stay honest
+// if one is added later.
+thread_local AllocStats alloc_stats;
+thread_local bool       alloc_counting = false;
+
+class CountAllocations
+{
+public:
+  CountAllocations()
+  {
+    alloc_stats    = AllocStats{};
+    alloc_counting = true;
+  }
+  ~CountAllocations() { alloc_counting = false; }
+
+  AllocStats
+  stats() const
+  {
+    return alloc_stats;
+  }
+};
+
+#if defined(__linux__)
+constexpr bool ALLOC_COUNTING_AVAILABLE = true;
+#else
+constexpr bool ALLOC_COUNTING_AVAILABLE = false;
+#endif
+
+} // namespace
+
+#if defined(__linux__)
+#include <dlfcn.h>
+
+// Interpose the system allocator. Defining these in the executable takes 
precedence over
+// libc for every caller in the process, which is what makes the count cover 
PCRE2's own
+// allocations as well as the wrapper's.
+namespace
+{
+using malloc_fn  = void *(*)(size_t);
+using free_fn    = void (*)(void *);
+using calloc_fn  = void *(*)(size_t, size_t);
+using realloc_fn = void *(*)(void *, size_t);
+
+malloc_fn  real_malloc  = nullptr;
+free_fn    real_free    = nullptr;
+calloc_fn  real_calloc  = nullptr;
+realloc_fn real_realloc = nullptr;
+
+// dlsym() itself can allocate while the real pointers are still being 
resolved. Hand
+// those few allocations out of a static buffer rather than recursing.
+alignas(std::max_align_t) char bootstrap_buffer[16384];
+size_t bootstrap_used = 0;
+bool   resolving      = false;
+
+bool
+from_bootstrap(void *p)
+{
+  return p >= static_cast<void *>(bootstrap_buffer) && p < static_cast<void 
*>(bootstrap_buffer + sizeof(bootstrap_buffer));
+}
+
+void *
+bootstrap_alloc(size_t size)
+{
+  size_t const aligned = (size + alignof(std::max_align_t) - 1) & 
~(alignof(std::max_align_t) - 1);
+  if (bootstrap_used + aligned > sizeof(bootstrap_buffer)) {
+    return nullptr;
+  }
+  void *p         = bootstrap_buffer + bootstrap_used;
+  bootstrap_used += aligned;
+  return p;
+}
+
+void
+resolve_real_allocators()
+{
+  if (real_malloc != nullptr || resolving) {
+    return;
+  }
+  resolving    = true;
+  real_malloc  = reinterpret_cast<malloc_fn>(dlsym(RTLD_NEXT, "malloc"));
+  real_free    = reinterpret_cast<free_fn>(dlsym(RTLD_NEXT, "free"));
+  real_calloc  = reinterpret_cast<calloc_fn>(dlsym(RTLD_NEXT, "calloc"));
+  real_realloc = reinterpret_cast<realloc_fn>(dlsym(RTLD_NEXT, "realloc"));

Review Comment:
   Good catch, and it is real on both counts.
   
   `resolve_real_allocators()` assigned `real_malloc` first and returned early 
on it, so a
   nested allocation during the remaining three lookups re-entered a wrapper, 
found the early
   return already satisfied, and called through a still-null `real_free`. The 
second point
   holds too: `realloc` would have handed a `bootstrap_alloc()` pointer to the 
system
   allocator, which never issued it.
   
   Fixed in f70992eb6e. All four are resolved into locals and published 
together, with
   `real_malloc` last since that is the pointer the early return tests, so the 
table is never
   half visible. Each wrapper still tests its own pointer rather than trusting 
that one: while
   resolving, `free` leaks rather than calling null, which is the right trade 
for the handful
   of blocks the loader turns over at startup, and `realloc` moves a bootstrap 
block with
   `malloc` plus `memcpy` instead of passing it on.
   
   Re-verified on Linux: the counters still read zero allocations for every 
match path and six
   for a compile, which is what they read before.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to