feat(major): [sc-23696] replace jemalloc dependency with custom malloc interposer#333
feat(major): [sc-23696] replace jemalloc dependency with custom malloc interposer#333supersonicbyte wants to merge 43 commits into
Conversation
hassila
left a comment
There was a problem hiding this comment.
Some initial feedback, will ping you offline about the initial bootstrap problem...
|
Also I see a difference in which metrics are output between main and this branch, e.g. : Main: this PR: Got some additional malloc related metrics there, would be good to understand root cause. |
|
(that was from running |
|
Also one issue when testing with package-benchmarks-samples repo with lost capture of allocation: Main gives: PR: |
|
(Memory (allocated resident) (K) was missing from the PR output too?) |
|
This test also misses capture: PR: |
|
And this test should show a leak, but does not on the PR: PR: |
Thanks for the feedback! That's strange will look into it.. |
|
This one failed to find the Malloc/free delta leak on Linux: |
|
For Linux: cp .build/x86_64-unknown-linux-gnu/debug/libMallocInterposerC-tool.so .build/x86_64-unknown-linux-gnu/debug/libMallocInterposerC.so
cp .build/x86_64-unknown-linux-gnu/debug/libMallocInterposerSwift-tool.so .build/x86_64-unknown-linux-gnu/debug/libMallocInterposerSwift.so(need to rebuild in between too, so build, first copy, build, second copy, ...) |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #333 +/- ##
==========================================
+ Coverage 68.85% 69.49% +0.65%
==========================================
Files 33 34 +1
Lines 3152 3245 +93
==========================================
+ Hits 2170 2255 +85
- Misses 982 990 +8
... and 2 files with indirect coverage changes
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
Currently blocked by this: swiftlang/swift#87696 (comment) |
|
Pull request had an unknown failure |
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@claude review |
Description
This PR replaces
jemallocwith a custom malloc interposer. The interposer logic is mostly inspired by the great work done by theswift-nioteam.On macOS/Darwin we ulitise the interposing feature of dydl while on Linux we use
LD_PRELOADin order to interpose.The interposition logic is written in C and located in the
MallocInterposerCpackage.For actual interaction with the interposition is done through a wrapper package
MallocInterposerSwift, which exposes a simple interface tohookandunhookthe interposition logic and also to actually count the statistics we need.The interposition requirers the the interposer library to be linked dynamically, since that's crucial to the nature of how the interposition works (both on Linux and Darwin). The SwiftPM Command plugin
BenchmarkCommandPluginhas a dependency onBenchmarkwhich has a dependency on the interposer libs. This causes a problem for SwiftPM and intitial tiggers ofswift package benchmarkcommand will fail.The issue is described in more detail here.
The current workaround is to trigger the
swift package benchmarkcommand once and let it fail. Then to copy thelibMallocInterposerC-tool.dylibandlibMallocInterposerSwift-tool.dyliband name them without the-toolsuffix.e.g.
The same applies for Linux but the build location will be different and the dynamic library extension is
.so.Since jemalloc is removed the
mallocSmallandmallocLargemetrics are removed. New metricsmallocByteCountandfreeTotalCountare introducted.How Has This Been Tested?
When running the test the executable is being linked statically to the test target. I didn't discover a way yet to dynamically link the interposer in order for it to work inside tests.
Minimal checklist:
DocCcode-level documentation for any public interfaces exported by the package