Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inlines: make this.matches a dense rather than sparse array. #24

Merged
merged 1 commit into from
Jan 6, 2023

Conversation

jgm
Copy link
Owner

@jgm jgm commented Jan 6, 2023

This yields a 10-15% speeed boost in my tests. See #13.

However, we now fail a couple of smart quote tests. I need to look into that.

@jgm jgm mentioned this pull request Jan 6, 2023
This yields a significant performance boost. See #13.

Of course, the speedup depends on the input. This will mostly
affect texts with significant strings of inlines (longer paragraphs
rather than just a few words).  But on those, we are more than
twice as fast with this change.

Here are some inline-heavy benchmarks for comparison:

This branch:
parse inline-em-flat.dj x 24,362 ops/sec ±0.26% (95 runs sampled)
parse lorem1.dj x 13,244 ops/sec ±0.29% (101 runs sampled)
parse inline-em-nested.dj x 25,121 ops/sec ±0.23% (96 runs sampled)
parse inline-em-worst.dj x 27,437 ops/sec ±0.66% (92 runs sampled)
parse readme.dj x 2,529 ops/sec ±0.55% (96 runs sampled)

Current main:
parse inline-em-flat.dj x 25,057 ops/sec ±0.26% (99 runs sampled)
parse lorem1.dj x 5,846 ops/sec ±0.26% (98 runs sampled)
parse inline-em-nested.dj x 25,584 ops/sec ±0.26% (100 runs sampled)
parse inline-em-worst.dj x 26,859 ops/sec ±0.29% (98 runs sampled)
parse readme.dj x 1,144 ops/sec ±0.43% (96 runs sampled)

Thanks to @matklad for identifying this as a bottleneck
and suggesting the general direction of the improvement.
@jgm jgm marked this pull request as ready for review January 6, 2023 21:32
@jgm
Copy link
Owner Author

jgm commented Jan 6, 2023

OK, this seems to work well now. I'm getting > 2X speedup in the most favorable benchmarks!

@jgm
Copy link
Owner Author

jgm commented Jan 6, 2023

On a real-world case parsing goes from 55ms too 40ms.

@matklad
Copy link
Contributor

matklad commented Jan 6, 2023

Nice!

@jgm jgm merged commit 832a912 into main Jan 6, 2023
@jgm jgm deleted the altinlines branch January 6, 2023 22:10
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.

2 participants