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

Optimized Parse.Sequence(...) combinator #94

Merged
merged 1 commit into from
Dec 2, 2020

Conversation

nblumhardt
Copy link
Member

Parse.Sequence() is a non-allocating alternative to Then(). It avoids construction of nested parsers by flattening out the sequence.

A comparison from the benches:

        static readonly TextParser<(char, char, char)> ThenParser =
            Character.Digit.Then(first => 
                Character.Digit.Then(second =>
                    Character.Digit.Then(third => Parse.Return((first, second, third)))));

        static readonly TextParser<(char, char, char)> SequenceParser =
            Parse.Sequence(
                Character.Digit,
                Character.Digit,
                Character.Digit)
                .Select(t => t); // Even up the work done

Results from running this on a small input:

Method Mean Error StdDev Ratio RatioSD Gen 0 Gen 1 Gen 2 Allocated
ApplyThen 519.6 ns 10.233 ns 13.31 ns 1.00 0.00 0.1473 - - 464 B
ApplySequence 314.4 ns 6.249 ns 10.09 ns 0.61 0.02 - - - -

It's not expected that Sequence would always replace Then, but the difference in allocs will help boost possible "v3" perf quite a bit, in conjunction with a few other planned changes.

The PR is setting up for a longer, deeper 3.0 where some breaking changes are expected. Rather than start depending on System.ValueTuple I made the (possibly rash) move to drop every target except .NET Standard 2.0. We can reconsider this down the track, but for now it'll keep the work-in-progress clean and simple.

The signature of Sequence() could be extended to include an additional "selector" argument, avoiding the need for a chained Select() call to unpack the tuple. Current C# limitations make lambdas accepting tuples a bit ugly - but if there's any hope this will improve, my preferences tip towards the cleaner signature used in the current PR version.

@nblumhardt nblumhardt added this to the 3.0.0 milestone Jun 12, 2019
@nblumhardt nblumhardt merged commit f04deea into datalust:dev Dec 2, 2020
@nblumhardt nblumhardt mentioned this pull request Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant