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

Speed ups #75

Merged
merged 4 commits into from
Jun 21, 2018
Merged

Speed ups #75

merged 4 commits into from
Jun 21, 2018

Conversation

SSJohns
Copy link

@SSJohns SSJohns commented Jun 21, 2018

Hey these changes are a part of a heap profiling pass that we've been working on. It basically identifies the most common things that are slowing down the parser or things that are being allocated but never used.

Quick run-down:

876e8b9: These vecs are allocated with a certain size, but it is often the case that they are never used. We let the allocator give them size in the case that they are used.

67faebf: Often seek is 0 so we don't need to do the memory copying and overwriting unless it's different.

f9b0f9d: The reference inlines noramlly don't have a delimiter, so we start the delimiter with zero capacity.

2e00ec7: Big win, often scanners.rs creates a large reference structure when we could just check the very first character. Since these are called so often this eliminates a large amount of unneeded allocations.

Benches are done on average of 3.
Bench speedup: (9,511,724 + 9,335,745 + 9,226,028)/3 = 9357832
Bench master: (12,177,662 + 12,865,365 + 12,590,586)/3 = 12544537

SSJohns added 4 commits June 21, 2018 16:16
In many cases these vecs are never used so initing them with
a capacity will waste space and allocations. Also the
content.clear() in these cases don't seem to be doing
anything
@kivikakk
Copy link
Owner

This is amazing, thank you! I'm a big fan of the scanner change; the ugliness introduced by essentially having the start of the parsing rule duplicated is unfortunate, but necessary to get these perf gains.

@kivikakk kivikakk merged commit fa151b6 into kivikakk:master Jun 21, 2018
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