Conversation
Port of the parser half (Option D) of ms/75-resolve-excessive-memory-use-
part-3's 986bbad onto current master, split from that commit's other,
independent half (dependency-retention fixes, already ported as
v2-part-4) and its opt-in disk-spill feature (deferred to v2-part-7).
Clean, direct port -- lib/sprig/{parser,source}.rb and sprig.gemspec were
untouched by every commit master gained after the original stack branched,
so no integration work was needed here.
Parser::Yml/Json no longer fully parse a file into memory via
YAML.load/JSON.load. Each now runs a Psych::Parser/Oj::ScHandler callback
handler (new EventHandler classes) that reads directly off the source IO,
rewinding between an eager pass that captures `options:` (small, cheap to
build fully) and a lazy pass -- deferred until the returned Enumerator is
actually iterated -- that streams `records:` one row at a time, matching
by key name (not position) since `options:`/`records:` can appear in
either order. Parser::Csv reverted to streaming CSV.foreach directly. This
only works because Source#data no longer closes the IO the instant #parse
returns -- it now closes once the records enumerator is actually
exhausted (or parsing fails). Non-rewindable custom :source IOs (a real
pipe, verified against Errno::ESPIPE on #rewind) fall back to buffering
once, same as before. Adds `oj` as a runtime dependency (JSON streaming
needs Oj::ScHandler; Psych, used for YAML, is already part of Ruby's
stdlib).
Full spec suite passes on the default config (201 examples) and all five
Appraisals (rails-7.2/8.0/8.1, mongoid-8/9); standardrb clean.
Measured on this stack's own harness (Postgres) -- **this stage's
real-world impact turned out to be substantially larger than the original
investigation's own ranking suggested**, where it was measured only in
combination with the dependency-retention fixes and characterized as
mattering mainly for very large single files:
small (N=1,000): 82.1 MB -> 75.0 MB peak RSS (-8.6%)
medium (N=10,000): 234.1 MB -> 123.1 MB peak RSS (-47.4%)
large (N=100,000): 1,364.4 MB -> 590.4 MB peak RSS (-56.7%), single run
The medium and large drops are not typos -- confirmed reproducible across
3 runs each at medium (part-5: 215-276 MB; part-6 (this stage): 125-133
MB, non-overlapping) and mechanistically expected once isolated cleanly:
`YAML.load(data_io)` builds the *entire* parsed file -- every row of
every one of the 5 models -- as one big nested Ruby structure before
Factory ever converts the first row into a Descriptor. That whole
intermediate structure is gone once parsing streams a row at a time
straight into Descriptor construction. This effect scales with file
size, which is exactly why it shows up as modest at 1K and dominant at
10K/100K -- the original investigation's combined C+D measurement, plus
testing mostly at the same fixed scale as everything else in that
document, diluted this into looking like a secondary, situational
optimization. This is the single largest contributor of any stage in the
whole stack at both the medium and large tier.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Coverage is 99.043% — ms/75-resolve-memory-version-3-part-6 into ms/75-resolve-memory-version-3-part-5. No base build found for |
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
2 participants
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.
Ticket
#75
Resolve Sprig's excessive memory use when loading data.
Changes in this PR:
NOTE: This is Part 6 of 7 Parts. See the benchmark/README.md file for the complete explanation (it is unchanged across all stacked branches).
NOTE: Most of the content was authored by Claude Sonnet 5 under human supervision. Purely human-authored edits have their own commit.