Backport call-site aware memory tracking to release-7.4 (PR #13344) - #13818
Open
gxglass wants to merge 1 commit into
Open
Backport call-site aware memory tracking to release-7.4 (PR #13344)#13818gxglass wants to merge 1 commit into
gxglass wants to merge 1 commit into
Conversation
Backport of PR apple#13344 ("call-site aware memory tracking", main commit aa705c8) to the release-7.4 line. Adds a sampled, always-compiled, knob-controlled per-call-site memory tracker (flow/MemoryTracker.{cpp,h}) hooked into the three FDB-owned allocation paths: the global operator new/delete set, FastAllocator<Size>, and ArenaBlock::create/destroyLeaf. A periodic dump from SystemMonitor emits MemoryTrackerSite / MemoryTrackerSummary TraceEvents. Prod default is off; simulation samples 1-in-10 so the path is exercised. The whole feature is gated at compile time by FDB_MEMORY_TRACKER (on by default; build with -DFDB_MEMORY_TRACKER=OFF to compile it out). The replaced global operator new / operator delete (and the legacy ALLOC_INSTRUMENTATION accounting variants) are moved into fdbserver/GlobalNewDelete.cpp so the interposition is compiled into the fdbserver executable only, never into flow / libfdb_c / the client bindings. Verified with nm: the strong operator-new/delete definitions are present in the fdbserver binary and absent from libfdb_c.so and libflow.a. Differences from the main-branch PR, due to release-7.4 divergence: - operator-new removal and memTrackerInit() wiring applied to fdbserver/fdbserver.actor.cpp (main: fdbserver.cpp); knob re-init uses g_knobs.initialize() rather than initializeServerKnobs(). - forceLinkMemoryTrackerTests() wired into fdbserver/workloads/UnitTests.actor.cpp (main: UnitTests.cpp). - AGENTS.md added, adapted to 7.4 (flat fdbserver/ layout, ServerKnobs under fdbclient/, fdbserver -r unittests, absent design docs dropped). - Omitted: design/memory-tracker.md, the fdbserver/bench microbenchmark (needs main's benchmark harness, absent from 7.4), and the two contrib/mako_ab_*.py scripts (need contrib/mako_storage_bench.sh, absent from 7.4). Testing: - Build clean (Release), all targets. - All 14 /flow/MemoryTracker/ unit tests pass; ran 1000x with randomized seeds, 1000/1000 passed, 0 failures. - 100K Joshua correctness run, clean: 20260805-213736-gglass-36215d1242f71327 compressed=True data_size=41598074 duration=4226513 ended=100000 fail_fast=10 max_runs=100000 pass=100000 priority=100 remaining=0 runtime=0:41:34 sanity=False started=100000 stopped=20260805-221910 submitted=20260805-213736 timeout=5400 username=gglass
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Result of foundationdb-pr-macos-m1 on macOS 14.x
|
Contributor
Result of foundationdb-pr-clang on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-clang-arm on Linux RHEL 9
|
Contributor
Result of foundationdb-pr on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-macos on macOS 14.x
|
Contributor
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of PR #13344 ("call-site aware memory tracking", main commit aa705c8) to the release-7.4 line.
Adds a sampled, always-compiled, knob-controlled per-call-site memory tracker (flow/MemoryTracker.{cpp,h}) hooked into the three FDB-owned allocation paths: the global operator new/delete set, FastAllocator, and ArenaBlock::create/destroyLeaf. A periodic dump from SystemMonitor emits MemoryTrackerSite / MemoryTrackerSummary TraceEvents. Prod default is off; simulation samples 1-in-10 so the path is exercised. The whole feature is gated at compile time by FDB_MEMORY_TRACKER (on by default; build with -DFDB_MEMORY_TRACKER=OFF to compile it out).
The replaced global operator new / operator delete (and the legacy ALLOC_INSTRUMENTATION accounting variants) are moved into fdbserver/GlobalNewDelete.cpp so the interposition is compiled into the fdbserver executable only, never into flow / libfdb_c / the client bindings. Verified with nm: the strong operator-new/delete definitions are present in the fdbserver binary and absent from libfdb_c.so and libflow.a.
Differences from the main-branch PR, due to release-7.4 divergence:
Testing, correctness:
20260805-213736-gglass-36215d1242f71327 compressed=True data_size=41598074 duration=4226513 ended=100000 fail_fast=10 max_runs=100000 pass=100000 priority=100 remaining=0 runtime=0:41:34 sanity=False started=100000 stopped=20260805-221910 submitted=20260805-213736 timeout=5400 username=gglass
Testing, performance:
Attachments show saturation throughput of {{compiled out vs compiled-in but disabled}, {compiled in, disabled vs compiled in, enabled at 1% sampling}}; and p50 latency of {{compiled out vs compiled-in but disabled}, {{compiled-in, disabled vs compiled-in, enabled at 1% sampling}}.
Summary:
The intent is to be able to compile in by default and ship that, and only enable when needed (e.g. interesting end to end test scenarios, or for leak debugging as a last resort in actual production).
FDB Memory Tracker A_B_ release-7.4 tracker compiled out vs in (tracking off).pdf
FDB Memory Tracker A_B_ off vs 1_100 sampling.pdf
FDB Memory Tracker A_B_ compiled-out vs compiled-in-disabled latency @ 2500 tps.pdf
FDB Memory Tracker A_B_ latency @ 2500 tps (sub-saturation, 3 runs_arm).pdf