Repository navigation
fix(models): reserve storm reports only for features that become reports - #28
Merged
Merged
Conversation
Reddimus
added a commit
that referenced
this pull request
Sep 25, 2026
Sets the version to 0.4.2 in `include/spc/version.hpp`, dates the changelog section, and points the README's `GIT_TAG` at v0.4.2, which `make test-consumers` checks. The `find_package(spc 0.4 REQUIRED)` line stays, because 0.4.x is compatible. v0.4.2 contains the performance and correctness work measured in each PR: #16 and #28 (the storm report list is sized once, and only for real reports), #19 (numeric strings parse the same way on every platform), #21 (the typed ArcGIS queries parse each page once, about half the CPU and allocations), #24 (the `parse_*` functions stay inside their `string_view`), #25 (error docs), plus the CI and test-tooling changes in #22, #26, and #29. No public API changes. After merge, tagging `v0.4.2` runs the release workflow. It checks the tag against `version.hpp`, runs the tests and consumer checks, and publishes the changelog section as the release notes.
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
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.
Summary
#16 made
parse_storm_reportsreserve its list from the number offeatures, on the grounds that every IEM feature becomes a report. It skips elements withoutproperties, though, so a body padded with anything else reserved memory it never used, and the empty result kept it: about 2.6 MB for 10,000 zeros, and about 500 MiB for a 4 MB body of two million. It now counts the elements that will become reports and reserves exactly that. The spc-cpp release review found this.Closes #27
The bug never shipped (v0.4.1 grows the list as it goes), so there's no CHANGELOG entry; #16's entry still describes the behavior.
Benchmarks
BM_Parse/storm_reportson the 624-report IEM fixture, main (153f76e) against this branch, macOS with Apple Clang 21. The counting pass costs 0.4% more instructions (20,442,098 → 20,528,443). Allocations, bytes allocated, and memory held by the result are unchanged, so #16's savings stand.Testing
New
Models.StormReportsReserveOnlyForReportsparses 10,000 zeros and expects the empty result to keep no capacity. On main it keeps 10,000.Models.StormReportsParseIemLsrstill checks that a real day's list is sized with one reservation.macOS:
make testpasses 130/130 andmake lintis clean.make testandmake lintpassCHANGELOG.md: no entry, since the bug is unreleasedNo public API changes