Conversation
machine:heap says a leak exists. It does not say where, and that has been the
slow part of every leak fixed recently: each one had correct-looking ownership
on every path around it, so reading code did not find them.
This records every live allocation with the address that asked for it, and
reports them grouped by that address. Slots are addressed directly from the
pointer rather than searched, because record and forget sit in the allocator
path and a linear scan there is not affordable once every allocation is
tracked. Four ways per set: a plain direct-mapped table lost a noticeable
number of records to collisions on real runs, and when a set is genuinely full
the oldest entry is dropped and counted, so an under-reporting table says so.
operator new and malloc both funnel through get_mem(), so the tracker
re-attributes each block to the real caller there; without that every
allocation would read as the wrapper.
Nothing is built unless HEAP_TRACK is defined: the entry points compile to
nothing, no table is reserved, and the allocator does no extra work. The
Makefiles gain $(EXTRA_DEFINES) so a diagnostic image is
make u64_no_esp EXTRA_DEFINES=-DHEAP_TRACK=1
The FreeRTOS heap itself takes four guarded lines and no logic.
tests/tools/heap_diff.py is the part that finds leaks rather than merely
listing allocations. A live block is not a leaked block, and on real firmware
most outstanding blocks are legitimate, so it runs the same work twice and
reports which callers grew between rounds. Reading the endpoint once and
treating the large entries as leaks is how you fix the wrong thing; that
mistake nearly attributed the FileInfo leak to the mount cache, because the
totals looked right for it.
Review feedback on GideonZ#776: what the endpoint reports is outstanding allocations grouped by the caller that made them, not FreeRTOS heap blocks. "Blocks" named the allocator's internal structure rather than the thing being measured, which is the sort of implementation detail an API should not leak. GET /v1/machine:heap_allocations PUT /v1/machine:heap_allocations_reset The payload follows the same rename, since it carried the same wrong noun: live_blocks becomes live_allocations, and each caller line reads "N allocations" rather than "N blocks". heap_diff.py parses that line, so it moves with it. The tracker's internal names are untouched. Inside the allocator they really are blocks.
Contributor
Author
|
Corrected HEAP_TRACK U64 artifact downloaded and verified for PR #776 hardware testing; temporary build PR no longer needed. |
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
1 participant
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.
Temporary diagnostic rebuild for PR #776 after the caller-list fix. Enables HEAP_TRACK only in the U64 CI command; this PR will be closed after its artifact is downloaded.