-
Notifications
You must be signed in to change notification settings - Fork 54
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
introduce hasKnownResult to optimize several parsers. #348
Conversation
Codecov Report
@@ Coverage Diff @@
## main #348 +/- ##
==========================================
+ Coverage 96.56% 96.91% +0.34%
==========================================
Files 9 9
Lines 1049 1103 +54
Branches 94 98 +4
==========================================
+ Hits 1013 1069 +56
+ Misses 36 34 -2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be interesting to see new benchmark numbers.
@johnynek could you rebase this? |
Yes! I will do. Thanks for the review. Note, this optimization will mostly apply to flatMap, and most parser don't need flatMap (e.g. our json benchmarks don't use flatMap). But still, I thought it was worth trying to limit the worst case. |
@regadas let me know if this looks good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Often, we can statically see what the result value will be if a parser succeeds.
If we know this, can optimize map, select and flatMap.
In particular, we can avoid reallocating a parser inside each flatMap.
Most users should avoid using flatMap anyway, but in a library function, you might need flatMap but want to get the optimization when the user passes you a constant parser.