Copilot commented on code in PR #13670:
URL: https://github.com/apache/trafficserver/pull/13670#discussion_r3997658285
##########
tools/benchmark/CMakeLists.txt:
##########
@@ -57,3 +57,11 @@ target_include_directories(benchmark_HuffmanDecode PRIVATE
${CMAKE_SOURCE_DIR}/l
add_executable(benchmark_ascii_tolower benchmark_ascii_tolower.cc)
target_link_libraries(benchmark_ascii_tolower PRIVATE Catch2::Catch2WithMain
ts::tscore)
+
+add_executable(benchmark_Regex benchmark_Regex.cc)
+target_link_libraries(benchmark_Regex PRIVATE Catch2::Catch2WithMain
ts::tsutil)
+if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+ # The allocation counters interpose the system allocator, which needs the
real
+ # symbols from the next object in the search order.
+ target_link_libraries(benchmark_Regex PRIVATE ${CMAKE_DL_LIBS})
Review Comment:
`RTLD_NEXT` is a GNU extension and glibc exposes it from `<dlfcn.h>` only
when `_GNU_SOURCE` is defined. This target does not add that definition, so the
Linux benchmark fails to compile with `RTLD_NEXT` undeclared; define
`_GNU_SOURCE` for this target (or before any system header).
##########
tools/benchmark/benchmark_Regex.cc:
##########
@@ -0,0 +1,686 @@
+/** @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, because the match data comes out of the caller's own
buffer. The
+ interpreter is the exception: it allocates a backtracking frames vector
through the same
+ allocator, so a match that runs interpreted does show up in the count.
+
+ 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 <cstddef>
+#include <cstdint>
+#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"
+
+#define PCRE2_CODE_UNIT_WIDTH 8
+#include <pcre2.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.
+//
+// Each block is preceded by a header holding its size, so a realloc of one
can copy the
+// old contents rather than silently returning uninitialised storage. The
header is one
+// max_align_t wide so the pointer handed back keeps the alignment malloc
promises.
+constexpr size_t BOOTSTRAP_HEADER = alignof(std::max_align_t);
+static_assert(BOOTSTRAP_HEADER >= sizeof(size_t), "the bootstrap header must
hold a size");
+
+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));
+}
Review Comment:
`from_bootstrap()` uses relational comparisons between an arbitrary
allocator pointer and an element of `bootstrap_buffer`. For a real allocation
those pointers are unrelated objects, so C++ does not define the ordering; a
false positive would make `free()` leak the allocation or make `realloc()` read
a bogus bootstrap header. Compare their integer addresses (this block is
already Linux-specific) instead of using pointer ordering.
--
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]
