Skip to content

Conversation

@jkoritzinsky
Copy link
Contributor

Depends on #1667

  • What does the pull request do?
    Change SelectorGrammar to use the Reader class used in binding path parsing (moved up to Avalonia.Markup in Allow LINQ Expressions for Binding Expressions #1667). Also, update Reader to be a ref struct with a ReadOnlySpan<char> to represent the member.

I've included some perf numbers below:

Sprache:
               Method |     Mean |    Error |   StdDev |   Gen 0 | Allocated |
--------------------- |---------:|---------:|---------:|--------:|----------:|
 ParseComplexSelector | 90.55 us | 1.764 us | 2.167 us | 52.2461 |  80.41 KB |

No Sprache:
               Method |     Mean |     Error |    StdDev |  Gen 0 | Allocated |
--------------------- |---------:|----------:|----------:|-------:|----------:|
 ParseComplexSelector | 3.921 us | 0.0695 us | 0.0879 us | 1.4420 |   2.22 KB |
Span-ified:
               Method |     Mean |     Error |    StdDev |  Gen 0 | Allocated |
--------------------- |---------:|----------:|----------:|-------:|----------:|
 ParseComplexSelector | 3.966 us | 0.0650 us | 0.0576 us | 1.1978 |   1.84 KB |

Sprache is the code in master. No Sprache is a Selector parser based on the Reader class that still uses strings and StringBuilders. Span-ified is the version at the tip of this PR branch (1133c3e).

Checklist:

@jkoritzinsky jkoritzinsky requested a review from a team June 10, 2018 03:14
@jkoritzinsky
Copy link
Contributor Author

Looks like because of dotnet/android#1679 (comment) the Span-ified vesion is blocked on release of Visual Studio 15.8.

@jkoritzinsky
Copy link
Contributor Author

There's a few workarounds in here for our Android projects that I can move into our packages so users don't need to work around it as well, but this PR is no longer blocked on Xamarin.Android!

@grokys @jmacato can you take a review pass at this?

Copy link
Member

@jmacato jmacato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

Copy link
Member

@grokys grokys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (Looks GREAT to me)!

@jkoritzinsky jkoritzinsky merged commit 343efcd into AvaloniaUI:master Aug 16, 2018
@jkoritzinsky jkoritzinsky deleted the selector-parse-no-sprache branch August 16, 2018 17:53
@grokys grokys added this to the 0.7.0 milestone Apr 3, 2019
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.

3 participants