⚡🔎 Abridge SequenceSet#inspect output for more than 512 entries
#502
Conversation
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
nevans
changed the title
nevans
force-pushed
the
sequence_set/abridged-inspect
branch
from
a9f8422 to
ef3a6e7
Compare
When using SequenceSet to store mailbox state for mailboxes with many
millions of messages, the last thing you want to do is accidentally
generate a string and output that!
So this establishes a maximum number of entries to output as an inspect
string, currently 512. Above that number, we output only a much smaller
number of entries from the head and tail, currently 16 from each end.
When the sequence set has an internal denormalized string, we use
regexps to grab the first and last groups of entries from that.
A note on the implementation: At first, I used a simple Regexp match
anchored to `\z` to grab the tail entries from the string. But that was
shockingly slow even when the string isn't extremely large.
Next I benchmarked replacing `Regexp#match` with `String#rpartition` and
that performed fine, using the same naive regexps. `String#rindex` was
also fine. They are both slower than `Regexp#match` anchored to `\A`,
but not too bad and not dramatically slower based on string size.
Then I benchmarked replacing `\d+` with `\d{0,10}` or `[1-9]\d{1,9}`,
and those both made the regexp fast... but `Regexp.linear_time?` doesn't
like nested quantifiers. Fully expanding both the `\d+` and the
`(entry){100}` keeps it fast _and_ satisfies `Regexp.linear_time?`.
But that regexp is UGLY! 🙁
nevans
force-pushed
the
sequence_set/abridged-inspect
branch
from
ef3a6e7 to
3585984
Compare
nevans
commented
Aug 28, 2025
| # | ||
| # This unrolls all nested quantifiers. It's much harder to read, but it's | ||
| # also the fastest out of all the versions I tested. | ||
| nz_uint32 = /[1-9](?:\d(?:\d(?:\d(?:\d(?:\d(?:\d(?:\d(?:\d(?:\d)?)?)?)?)?)?)?)?)?/ |
Collaborator
Author
There was a problem hiding this comment.
n.b:
# max digits in nz-number
(2**32 - 1).digits.count => 10
# to unroll /[1-9]\d{0,9}/
/[1-9]#{9.times.inject(//) { /(?:\d#{_1.source})?/ }.source}/ ==
/[1-9](?:\d(?:\d(?:\d(?:\d(?:\d(?:\d(?:\d(?:\d(?:\d)?)?)?)?)?)?)?)?)?/ =>
true
nevans
changed the title
SequenceSet#inspect output for more than 512 entries
nevans
added
the
sequence-set
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.
When using SequenceSet to store mailbox state for mailboxes with many millions of messages, the last thing you want to do is accidentally generate a string and output that!
So this establishes a maximum number of entries to output as an inspect string, currently 512. Above that number, we output only a much smaller number of entries from the head and tail, currently 16 from each end.
A note on the implementation
When the sequence set has an internal denormalized string, we use regexps to grab the first and last groups of entries from that.
At first, I used a simple Regexp match anchored to
\zto grab the tail entries from the string. But that was shockingly slow even when the string isn't extremely large.Next I benchmarked replacing
Regexp#matchwithString#rpartitionand that performed fine, using the same naive regexps.String#rindexwas also fine. They are both slower thanRegexp#matchanchored to\A, but not too bad and not dramatically slower based on string size. I found myself wishing for aString#rscanorString#rsplit. 😆Then I benchmarked replacing
\d+with\d{0,10}or[1-9]\d{1,9}, and those both made the regexp fast... butRegexp.linear_time?doesn't like nested quantifiers. Fully expanding both the\d+and the(entry){100}keeps it fast and satisfiesRegexp.linear_time?.But that regexp is UGLY! 🙁