Repository navigation
Memory regression tests - #275
alan-george-lk wants to merge 19 commits into
Conversation
2a21a8a to
d638cca
Compare
…ature-memory-regression-tests
There was a problem hiding this comment.
Note
Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.
🔍 Devin Review: 2 flags
Not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
stephen-derosa
left a comment
There was a problem hiding this comment.
overall looks great, thanks for adding these!
| matrix.e2e-testing && | ||
| (inputs.integration_repeat > 0 || | ||
| inputs.run_stress_tests || | ||
| (matrix.name == 'ubuntu-24.04-x64' && inputs.build_type == 'release')) |
There was a problem hiding this comment.
we are only running these on ubuntu x64 release builds?
There was a problem hiding this comment.
Fair callout, but some specific points:
- The original customer report was on Linux/x64
- The tools I built this with are linux-specific (the way RSS is tracked, and the
MALLOC_ARENAstuff), given the original customer report - Running on multiple platforms will clog up the GitHub summary page/take more CI time for likely negligible coverage differences
So I felt having a single run on the most representative platform for robotics/physical AI (resource constrainted linux) would be most applicable
| delegate.requireDisconnect(livekit::DisconnectReason::ClientInitiated); | ||
| } | ||
|
|
||
| // Exercises data-track publication, payload enqueueing, explicit unpublication, |
There was a problem hiding this comment.
these doc strings are great
…ature-memory-regression-tests
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
4 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| capture-clock | ||
| capture-pattern |
There was a problem hiding this comment.
🟡 All-scenario runs fail on standard builds
After a default release-tests build, --scenario all runs capture scenarios that the SDK cannot execute. Both fail, making the all-scenario run fail even when every standard scenario passes.
Learn more
The build defaults to capture disabled, but still produces the Linux memory tester. The script now appends both capture scenarios to all regardless of that build configuration. In a standard build, runCycle rejects each capture scenario, and the script returns a failure even if every other scenario passes.
Example: Run ./build.sh release-tests followed by source scripts/set-test-tokens.sh and scripts/memory-tester.sh --scenario all. The six standard scenarios can pass, but both capture scenarios fail with a capture-enabled-build error.
Recommended fix: Make all select capture scenarios only when the built SDK supports their respective sources, or provide a standard-only group and retain the existing behavior of all for non-capture builds. Keep explicit capture scenario requests failing clearly when unsupported.
Was this helpful? React with 👍 or 👎 to provide feedback.
This PR introduces a new test binary called
memory_testerfor Linux, which has the following features:CI is also updated to run this tester and provide a nice markdown table report of the results, and fail jobs that introduce changes that fail this.
Documentation is also updated around testing in general, as it was a bit out of date.
Results
Here are two FFI release results with the following params:
Prior release
livekit-ffi/v0.12.81results:Current FFI release results: