test: replace flaky StringMatcher.Memory with deterministic sizeof static_asserts

[Copilot]

StringMatcher.Memory was asserting tcmalloc page-level heap consumption against hard-coded byte ceilings (530176 + 5*8192, 15603776 + 10*8192). These constants are sensitive to size-class bucketing, span layout, and prior heap state — producing ~5/1000 flakes on gcc CI and requiring manual re-tuning on every tcmalloc/abseil/protobuf bump (PRs #37782, #43467). Simultaneously too tight (flakes on allocator noise) and too loose (can't detect small genuine regressions within the slack).

Changes

• Removed TEST_F(StringMatcher, Memory) and all EXPECT_MEMORY_LE / Memory::TestUtil::MemoryTest usage.

• Added compile-time static_asserts that enforce the structural invariant PR string-matcher: reduce per-matcher memory #37782 actually cared about — that each variant alternative carries only its required data and StringMatcherImpl doesn't retain the proto:

// String-holding alternatives: std::string + bool (padded) only.
static_assert(sizeof(Matchers::ExactStringMatcher) <= sizeof(std::string) + sizeof(void*), ...);
// ... same for Prefix, Suffix, Contains

// Pointer-holding alternatives: one unique_ptr only.
static_assert(sizeof(Matchers::RegexStringMatcher) <= 2 * sizeof(void*), ...);
static_assert(sizeof(Matchers::CustomStringMatcher) <= 2 * sizeof(void*), ...);

// StringMatcherImpl: 2 vtable ptrs + variant (largest alt + discriminant).
static_assert(sizeof(Matchers::StringMatcherImpl) <= sizeof(std::string) + 4 * sizeof(void*), ...);

Bounds use sizeof(std::string) and sizeof(void*) rather than raw byte counts, making them portable across libc++ (sizeof(std::string)==24) and libstdc++ (sizeof(std::string)==32).

• Added TEST_F(StringMatcher, SizeIsBounded) mirroring the static_asserts at runtime for test-runner visibility, with a comment block documenting the removed test's failure mode and the design intent.

• No BUILD changes needed — memory_test_utility_lib was never an explicit dep of matchers_test.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• dl.google.com
  • Triggering command: /build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/bin/java bazel(envoy) --add-opens=java.base/java.lang=ALL-UNNAMED -Xverify:none -Djava.util.logging.config.file=/build/bazel_root/base/javalog.properties -Dcom.google.devtools.build.lib.util.LogHandlerQuerier.class=com.google.devtools.build.lib.util.SimpleLogHandler$HandlerQuerier -XX:-MaxFDLimit -Djava.library.path=/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib:/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib/server:/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/ -Dfile.encoding=ISO-8859-1 -Duser.country= -Duser.language= -Duser.variant= -Xmx3g -DBAZEL_TRACK_SOURCE_DIRECTORIES=1 -Djavax.net.ssl.trustStore=/tmp/custom-cacerts -Djavax.net.ssl.trustStorePassword=changeit -jar /build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/A-server.jar --max_idle_secs=10800 --noshutdown_on_low_sys_mem --connect_timeout_secs=30 (dns block)
  • Triggering command: /home/REDACTED/.cache/envoy-bazel/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/bin/java bazel(envoy) --add-opens=java.base/java.lang=ALL-UNNAMED -Xverify:none -Djava.util.logging.config.file=/home/REDACTED/.cache/envoy-bazel/bazel_root/base/javalog.properties -Dcom.google.devtools.build.lib.util.LogHandlerQuerier.class=com.google.devtools.build.lib.util.SimpleLogHandler$HandlerQuerier -XX:-MaxFDLimit -Djava.library.path=/home/REDACTED/.cache/envoy-bazel/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib:/home/REDACTED/.cache/envoy-bazel/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib/server:/home/REDACTED/. 1/x64/lib/node_modules/npm/node_modules/@npmcli/run-script/lib/node-gyp-bin/node (dns block)
• ftpmirror.gnu.org
  • Triggering command: /build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/bin/java bazel(envoy) --add-opens=java.base/java.lang=ALL-UNNAMED -Xverify:none -Djava.util.logging.config.file=/build/bazel_root/base/javalog.properties -Dcom.google.devtools.build.lib.util.LogHandlerQuerier.class=com.google.devtools.build.lib.util.SimpleLogHandler$HandlerQuerier -XX:-MaxFDLimit -Djava.library.path=/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib:/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib/server:/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/ -Dfile.encoding=ISO-8859-1 -Duser.country= -Duser.language= -Duser.variant= -Xmx3g -DBAZEL_TRACK_SOURCE_DIRECTORIES=1 -Djavax.net.ssl.trustStore=/tmp/custom-cacerts -Djavax.net.ssl.trustStorePassword=changeit -jar /build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/A-server.jar --max_idle_secs=10800 --noshutdown_on_low_sys_mem --connect_timeout_secs=30 (dns block)
• mirror.bazel.build
  • Triggering command: /build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/bin/java bazel(envoy) --add-opens=java.base/java.lang=ALL-UNNAMED -Xverify:none -Djava.util.logging.config.file=/build/bazel_root/base/javalog.properties -Dcom.google.devtools.build.lib.util.LogHandlerQuerier.class=com.google.devtools.build.lib.util.SimpleLogHandler$HandlerQuerier -XX:-MaxFDLimit -Djava.library.path=/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib:/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib/server:/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/ -Dfile.encoding=ISO-8859-1 -Duser.country= -Duser.language= -Duser.variant= -Xmx3g -DBAZEL_TRACK_SOURCE_DIRECTORIES=1 -Djavax.net.ssl.trustStore=/tmp/custom-cacerts -Djavax.net.ssl.trustStorePassword=changeit -jar /build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/A-server.jar --max_idle_secs=10800 --noshutdown_on_low_sys_mem --connect_timeout_secs=30 (dns block)
• pkgconfig.freedesktop.org
  • Triggering command: /build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/bin/java bazel(envoy) --add-opens=java.base/java.lang=ALL-UNNAMED -Xverify:none -Djava.util.logging.config.file=/build/bazel_root/base/javalog.properties -Dcom.google.devtools.build.lib.util.LogHandlerQuerier.class=com.google.devtools.build.lib.util.SimpleLogHandler$HandlerQuerier -XX:-MaxFDLimit -Djava.library.path=/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib:/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib/server:/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/ -Dfile.encoding=ISO-8859-1 -Duser.country= -Duser.language= -Duser.variant= -Xmx3g -DBAZEL_TRACK_SOURCE_DIRECTORIES=1 -Djavax.net.ssl.trustStore=/tmp/custom-cacerts -Djavax.net.ssl.trustStorePassword=changeit -jar /build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/A-server.jar --max_idle_secs=10800 --noshutdown_on_low_sys_mem --connect_timeout_secs=30 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

Background

TEST_F(StringMatcher, Memory) in test/common/common/matchers_test.cc was added in PR #37782 to guard against per-matcher memory regressions in StringMatcherImpl (the std::variant-based redesign).

It asserts that the tcmalloc-reported consumedBytes() after constructing 1000 matchers stays under a hard-coded ceiling (530176 + 5 * 8192 for prefix, 15603776 + 10 * 8192 for regex).

This test is fundamentally broken and is the source of recurring gcc CI flakes (~5/1000 reproduction rate, observed continuously on main since at least the start of the current dev cycle). Specifically:

1. It measures tcmalloc page-level consumption, which is sensitive to size-class bucketing, span layout, page heap fragmentation, allocator release policy, and prior heap state — none of which are properties of StringMatcherImpl.
2. The hard-coded constants are derived from a single point measurement on the original author's machine and have to be hand-tuned every time tcmalloc/abseil/protobuf is bumped (see deps: bump up tcmalloc version #43467 which pushed 15038016 → 15603776).
3. The + N * 8192 slack is an explicit acknowledgement that the signal has page-sized noise — so the test is simultaneously too tight (flakes on allocator noise) and too loose (won't detect a small genuine regression of, say, +16 bytes/matcher × 1000 = 16 KiB, which fits inside the slack).
4. It's not hermetic — both sub-blocks share the same heap, prior tests' state leaks in.
5. It catches the wrong thing: the actual property string-matcher: reduce per-matcher memory #37782 wanted to enforce was structural ("ExactStringMatcher doesn't carry an unused regex pointer; StringMatcherImpl doesn't keep a copy of the proto"), not "the heap is N bytes after 1000 constructions."

The recent failure mode, for reference:

test/common/common/matchers_test.cc:497: Failure
Expected: (regex_consumed_bytes) <= (15603776 + 10 * 8192), actual: 15710496 vs 15685696

— i.e. only ~24 KiB over a ~16 MiB ceiling.

Goal

Remove the flaky EXPECT_MEMORY_LE-based test and replace it with deterministic checks that actually enforce the structural property #37782 was supposed to guard. Do not just delete the test — the per-matcher-size invariant is real and worth testing; the implementation just needs to test it deterministically.

What to do

In source/common/common/matchers.h the relevant types are:

• ExactStringMatcher — holds std::string exact_; bool ignore_case_;
• PrefixStringMatcher — holds std::string prefix_; bool ignore_case_;
• SuffixStringMatcher — holds std::string suffix_; bool ignore_case_;
• ContainsStringMatcher — holds std::string contents_; bool ignore_case_;
• RegexStringMatcher — holds Regex::CompiledMatcherPtr regex_;
• CustomStringMatcher — holds StringMatcherPtr custom_;
• StringMatcherImpl — wraps absl::variant<...> of the above, plus two vtable pointers (it inherits ValueMatcher and StringMatcher).

The point of the redesign was that sizeof(StringMatcherImpl) should be roughly sizeof(largest variant alternative) + variant discriminator + vtable pointers — and crucially, that no alternative carries fields it doesn't need (e.g. ExactStringMatcher must not hold a RegexMatcher pointer; StringMatcherImpl must not hold a copy of the original proto).

Replacement test design

In test/common/common/matchers_test.cc, delete the existing TEST_F(StringMatcher, Memory) and replace it with the following deterministic checks. Put them in a new test (or two) — pick whatever names fit local conventions, e.g. StringMatcher.SizeIsBounded and StringMatcher.AllocationsScaleLinearly.

1. Compile-time static_assert size bounds

Add static_asserts — either inside an anonymous-namespace test in matchers_test.cc, or directly in source/common/common/matchers.cc if that's preferred for visibility — that pin the size of each variant alternative and of StringMatcherImpl itself. Use generous-but-tight bounds expressed in terms of sizeof(std::string), sizeof(void*), etc., not raw byte counts, so they are portable across libc++/libstdc++ and 32/64-bit. For example:

// ExactStringMatcher / PrefixStringMatcher / SuffixStringMatcher / ContainsStringMatcher
// must not carry anything beyond a std::string + a bool (rounded up to alignment).
static_assert(sizeof(ExactStringMatcher) <= sizeof(std::string) + sizeof(void*),
              "ExactStringMatcher unexpectedly grew; if you added a field, "
              "verify it is actually needed for all exact matchers.");
static_assert(sizeof(PrefixStringMatcher) <= sizeof(std::string) + sizeof(void*),
              "PrefixStringMatcher unexpectedly grew; ...");
// ... same for Suffix, Contains.

// RegexStringMatcher / CustomStringMatcher should hold only one pointer-sized member.
...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

[repokitteh-read-only]

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #44699 was opened by Copilot.

see: more, trace.

[phlax]

cc @adisuissa

[phlax]

moved to #44701