Skip to content

Fix circular references that delay garbage collection in build_dsp - #230

Merged
iguinn merged 2 commits into
legend-exp:mainfrom
iguinn:main
Oct 16, 2026
Merged

iguinn merged 2 commits into
legend-exp:mainfrom
iguinn:main

Conversation

@iguinn

@iguinn iguinn commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

The ProcessingChain class and the various variable and processor managers circularly referenced each other; as a result, these classes do not garbage collect when their ref count hits 0, and instead they must wait for the circular garbage collector to run. This meant that when looping over channels in build_dsp, memory associated with the ProcessingChains and input buffers were allowed to accumulate for several iterations, creating a memory-leak like effect:

image `memray` trace of memory usage over time when processing many tables in a single file. The saw-tooth pattern in heap memory is caused by the accumulation of memory over many iterations, followed by the circular garbage collector running. The RSS memory stays high because numpy does not release the memory block it is using. In this test, the input buffers were responsible for most of the energy use. See [memray-flamegraph-build-dsp-leak.html](https://github.com/user-attachments/files/33471567/memray-flamegraph-build-dsp-leak.html)

This is fixed by using weakref.refs for the references to the ProcessingChain in each of the manager classes to break the circular reference. A test was added that when deling the Processing chain and input/output tables, they each actually get garbage collected.

image `memray` trace after the fix for the same process as above. The memory usage is more or less constant after running through one iteration, and ~half as much memory is used as above. See [memray-flamegraph-build-dsp-fixed.html](https://github.com/user-attachments/files/33471647/memray-flamegraph-build-dsp-fixed.html)
  • Also made the chan_config argument behave as advertised, skipping channels that are set to None or False.

AI disclosure: GLM-5.3 and qwen-3.8 were used with the opencode harness. They were used to identify the circular references, propose a test, and review the pull request before submitting

@codecov

codecov Bot commented Oct 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.12%. Comparing base (676b89d) to head (28b93a2).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
src/dspeed/build_dsp.py 50.00% 1 Missing ⚠️
src/dspeed/processing_chain.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #230      +/-   ##
==========================================
+ Coverage   69.10%   69.12%   +0.02%     
==========================================
  Files          62       62              
  Lines        4191     4201      +10     
==========================================
+ Hits         2896     2904       +8     
- Misses       1295     1297       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@iguinn
iguinn merged commit 14300fa into legend-exp:main Oct 16, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant