perf: reuse the encoding the after image already built - #59
Merged
Conversation
Copying the state encodes it and parses the result, then threw the encoded string away. complete then normalized and encoded the same hash a second time, only to learn its size. A committed turn therefore traversed the state three times and encoded it three times, where two of each are necessary. to_h_with_byte_size returns the copy beside the size of the encoding that produced it, and the commit enforces max_state_bytes against that size rather than by encoding again. The turn still fails with PayloadTooLarge before it opens its transaction, which a test now covers end to end, because nothing covered the hard limit before. Measured on SQLite against the released 0.14.3 tree, medians of five interleaved runs: 5.3% more committed messages per second at 13 KB of state, 16.1% at 116 KB, and 12.0% at 1 MB. solid-objects-js already measures the string it commits. This brings the gem to the same shape.
Greptile SummaryThe PR removes a redundant state serialization by carrying the encoded byte size alongside the deep-copied after-image.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness, security, or repository-rule issues identified. The reused value and byte size come from the same normalized JSON encoding, preserve the previous strict-greater-than size boundary, and are checked before the commit transaction. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Actor state after handler] --> B[Normalize and encode once]
B --> C[Parse independent after-image]
B --> D[Capture encoded byte size]
D --> E{Within max_state_bytes?}
E -- No --> F[Raise PayloadTooLarge]
E -- Yes --> G[Commit parsed after-image]
D --> H[Report large-state metric]
Reviews (1): Last reviewed commit: "perf: reuse the encoding the after image..." | Re-trigger Greptile |
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.
Follow-up to #58, which cut the per-turn state copies from three to two and
left one redundant traversal behind. This removes it.
Version 0.14.4.
What changed
State#to_hcopies the state by encoding it and parsing the result, thendiscarded the encoded string.
completethen normalized and encoded the samehash a second time, only to learn its byte size for
max_state_bytesand forsolid_objects.state.large.to_h_with_byte_sizereturns the copy beside the size of the encoding thatproduced it. A committed turn now traverses the state twice and encodes it
twice, against three of each before.
Serialization.deep_copy_with_byte_sizeis new;deep_copycalls it, soboth encode once.
max_state_bytesis enforced against the reported size rather than byencoding the state again. The turn still fails with
PayloadTooLargebeforeit opens its commit transaction.
Measured
Apple M5, Ruby 4.0.5, Rails 8.1.3.1, SQLite 3.53.2. One hot actor, 300 messages
per size, median of five runs, the two trees run one after the other in each
round, against the released
0.14.3.The empty-state row sits inside run-to-run variance.
Effects to review
Serialization.deep_copy_with_byte_sizeandState#to_h_with_byte_sizeare new.deep_copyandto_hkeep theirsignatures and return values.
same JSON encoding of the same normalized state that
dump_with_byte_sizeproduced.
Tests
Written first, each watched fail:
test_a_state_above_the_hard_limit_fails_its_message_and_commits_nothingisnew and passed against
mainbefore the change, deliberately: it pins themax_state_bytesbehavior this diff moves, which nothing covered before.The two copy-count tests now count
to_h_with_byte_sizeas well asto_h,because the after image comes from the new method; the property they assert,
one full copy after the handler, is unchanged.
Validation
Parity
solid-objects-jsalready measures the string it commits, so no port isneeded; this brings the gem to the same shape.