Complete rewrite of tokenization/parsing #72
Merged
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
This is a massively breaking change internally and to anything that depended on the internal tokenization/validation system. It is changed in many fundamental ways. The general public interface is about the same as it was with some more minor binary-breaking changes to methods and constructors.
This makes some of the helper methods in NoRobotsRfcHelper redundant but also simplifies handling of tokens in any code that uses RobotsFileTokenReader.
We can do this because there is no official spec here. We're not typically expecting there to be new lines in the data we received though.
Turnerj
force-pushed
the
replace-tokenizer
branch
from
cf7557f to
84a8d97
Compare
|
Performance improvements for Before
After
Results All with zero allocations! |
Ideally we probably should somehow combine duplicate rules but we weren't before so...
|
Current progress has a little bit of a regression - guessing it still is about that hand off to
|
|
After a bunch of back and forth, I realised rather than processing the characters directly in a span, I could process the bytes directly to avoid a bunch of overhead from going back and forth.
|
This avoids a bunch of UTF-8 conversion interop
|
Items left to do:
|
Hopefully this resolves the preview C# version issue
|
Initial benchmark for
|
|
More efficient processing of directives by using a mutable structure and dropping all of the LINQ processing.
|
This was referenced Aug 8, 2022
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.
This is a massively breaking change internally and to anything that depended on the internal tokenization/validation system. It is changed in many fundamental ways.
The general public interface is about the same as it was with some more minor binary-breaking changes to methods and constructors.
Closes #42, closes #71 and allows access to the data in #13
Before any changes
After the initial tokenization replacement
After
FromStreamactually processing it via a streamAfter
FromStringsharing the same code path asFromStreamFinal Benchmark Results for
RobotsFileParserFromStream: 95% faster, 94% fewer allocationsFromString: 92% faster, 94% fewer allocationsBefore any changes
Initial LINQ-heavy rewrite
Post-LINQ optimizations and performance improvements
FromRules: 92% faster, 95% fewer allocations