Skip to content

CI: fail on a RAM regression, not only on running out of RAM - #186

Merged
rtissera merged 3 commits into
masterfrom
ci-ram-regression-thresholds
Sep 10, 2026
Merged

CI: fail on a RAM regression, not only on running out of RAM#186
rtissera merged 3 commits into
masterfrom
ci-ram-regression-thresholds

Conversation

@rtissera

@rtissera rtissera commented Sep 6, 2026

Copy link
Copy Markdown
Owner

The per-codec RAM ceilings in the three check_budget.py scripts were sized as platform budgets — what the target can afford — so they sit two to eight times above what the code actually uses. That catches a catastrophe and nothing smaller.

Concretely: cd_cdlz used 99014 bytes before the in-place CD spread (#183) and uses 57905 now. The whole 41% win could be handed back tomorrow and the job would stay green, because 99014 is still under the 200000 ceiling.

Two thresholds, because there are two questions

REGRESSION sits about 10% over what the code uses today and answers did we just lose ground. PLATFORM stays exactly where it was and answers does it still fit. Both have to hold, and the output names which one failed:

ok             cd_cdlz    peak=  57905 regression=  64000 platform= 200000
REGRESSION     cd_cdlz    peak=  99014 regression=  64000 platform= 200000
OVER PLATFORM  cd_cdzs    peak= 350000 regression= 142000 platform= 320000

Where the 10% comes from

All seven codecs measured on all three targets, today, on master:

codec rv32imafc cortex-m33 hazard3 regression platform (rv32)
hd_zlib 19766 19766 19766 22000 100000
hd_zstd 107205 107197 107205 118000 150000
hd_lzma 27858 27858 27858 31000 80000
hd_huff 23219 23219 23219 26000 200000
cd_cdzl 49786 49786 49786 55000 200000
cd_cdzs 128795 128787 128795 142000 320000
cd_cdlz 57905 57905 57905 64000 200000

Three different ISAs agree exactly on five codecs and differ by 8 bytes on the other two, so peak heap here is a property of the allocation sizes rather than of the ABI. 10% is therefore generous headroom for allocator and toolchain drift, not a fudge factor.

It is not headroom for design changes. Raising one should happen in the commit that spends the memory, with a reason.

Checked

Against the real qemu logs from all three targets (all pass), against a log edited to put cd_cdlz back at 99014 (fails with REGRESSION where it previously passed), against an over-platform value, and against a missing codec.

The per-codec ceilings were sized as platform budgets - what the target can
afford - so they sit two to eight times above what the code actually uses.
That catches a catastrophe and nothing smaller. cd_cdlz used 99014 bytes
before the in-place CD spread and uses 57905 now; the whole 41% win could be
handed back and the job would stay green, because 99014 is still under the
200000 ceiling.

So there are two thresholds per codec now, because there are two questions.
REGRESSION sits about 10% over what the code uses today and answers "did we
just lose ground". PLATFORM stays where it was and answers "does it still
fit". Both have to hold; the output names which one failed.

The 10% is not a guess. All seven codecs were measured on all three targets:
rv32imafc, cortex-m33 and hazard3 agree exactly on five of them and differ by
8 bytes on the other two, so peak heap here is a property of the allocation
sizes rather than of the ABI. The headroom is for allocator and toolchain
drift, not for design changes - raise one deliberately, in the commit that
spends the memory, and say why.

Checked against the real qemu logs from all three targets, and against a log
edited to put cd_cdlz back at its pre-spread 99014: the job now fails with
REGRESSION where it previously passed.
rtissera and others added 2 commits September 10, 2026 09:30
They joined the budget firmware with the STREAMINFO block-size fix and had
platform ceilings but no regression threshold, so giving those bytes back
would not have failed the job - which is the whole point of this change.

Baselines taken on rv32imafc; the three targets agree to within 8 bytes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01846EhHuAFk5qvxwEA5Gq6y
@rtissera
rtissera merged commit 9233f21 into master Sep 10, 2026
37 checks passed
@rtissera
rtissera deleted the ci-ram-regression-thresholds branch September 10, 2026 07:56
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