perf: bench for pratt and shunting yard parsers#620
perf: bench for pratt and shunting yard parsers#62039555 wants to merge 47 commits intowinnow-rs:mainfrom
pratt and shunting yard parsers#620Conversation
|
Oh, interesting. If only there was a way to share an "allocator" between each recursive instance of the algorithm. If we provided a custom allocator, users could at least seed the allocator with an expected capacity. Granted, in that case, the user could specify |
|
I did try #[derive(Default)]
enum Stack<T> {
#[default]
Empty,
One(T),
Many(Vec<T>),
}and that saw no performance change despite my quick and dirt hacks showing the stack sizes were usually 0-2 items, up to 4 items for the benchmark. |
|
Capacity of 4 didn't make a difference |
|
If we had built-in parenthesis handling, then this would likely not be an issue. |
|
I'm leaning towards going the safety route. This level of nesting used in the benchmark is an extreme. |
|
Report (numbers from M2)
Small expressionsUnfortunately, this is not the case for small expressions such as Report (numbers from M2)
Without SmallVec, the performance degrades significantly: Report (numbers from M2)
And the performance is not much better without brackets: Report (numbers from M2)
|
|
I worry about pulling in a dependency just for one algorithm that most won't use, of coming up with an appropriate abstraction for a btw something I forgot to call out in the discussion of trade offs in this thread is that recursion is a more trivial solution for no_std than using the heap. Also, API wise, a pratt parser could return a Over at #618 (comment) you called out Double E Method. Whats your opinion on
|
That link, as I understand it, describes the same approach that I've implemented here and called it shunting yard. It's a modification of the original shunting yard but it can handle unary operators and can reject ill-formed expressions similar to the Pratt. We already have two algorithms with similar interfaces. I'm planning to create a comprehensive example, similar to the JSON one. This will be useful to have anyway. Something like parsing function calls or ternary operators. This might show the advantages and disadvantages of each approach |
Co-authored-by: Ed Page <eopage@gmail.com>
This feature was an overengineering based on suggestion "Why make our own trait" in winnow-rs#614 (comment)
works without it
…d be - based on review "Why allow non_snake_case?" in winnow-rs#614 (comment) - remove `allow_unused` based on "Whats getting unused?" winnow-rs#614 (comment)
until we find a satisfactory api based on winnow-rs#614 (comment) > "We are dumping a lot of stray types into combinator. The single-line summaries should make it very easy to tell they are related to precedence"
based on "Organizationally, O prefer the "top level" thing going first and then branching out from there. In this case, precedence is core." winnow-rs#614 (comment)
the api has an unsound problem. The `Parser` trait is implemented on the `&Operator` but inside `parse_next` a mutable ref and `ReffCell::borrow_mut` are used which can lead to potential problems. We can return to the API later. But for now lets keep only the essential algorithm and pass affix parsers as 3 separate entities Also add left_binding_power and right_binding_power to the operators based on winnow-rs#614 (comment)
I will write the documentation later
# Conflicts: # src/combinator/shunting_yard.rs
- require explicit `trace` for operators - fix associativity handling for infix operators: `1 + 2 + 3` should be `(1 + 2) + 3` and not `1 + (2 + 3)`
- ternary operator - function call - index
- fix failing tests related to the ternary operator and commas
# Conflicts: # src/combinator/mod.rs
updates from precedence.rs: - enum `Assoc` - associativity `Neither` - function pointers `fn()` instead of `dyn& Fn`
Update with feature-complete parsersI pushed an updated benchmark inside the example using the Without bumpalo
Reusing the bump arena
A new bump arena each time
|
|
It is hard to beat the application stack in terms of flexibility, platform support and speed. The recursion grows only with prefixes like |i: &mut Stateful<...>| {
if i.state.recursion > 99 {
return Err(...)
}
i.state.recursion +=1;
...
}.value((Right(20), |i: _, a: _, b:_ | {
i.state.recursion -= 1;
a + b
})) |
This commit: * Fixes errors due to winnow 0.7 migration * Adapts winnow-rs/winnow#620 to work outside winnow * Uncomments test cases For the migration steps, I've attached CHANGELOG.md line numbers for context. Use winnow-rs/winnow@73c6e05 for the version of CHANGELOG.md in winnow-rs/winnow, as the line numbers will inevitably change with time. The main migration steps are: * `PResult` replaced with `winnow::Result` (L153, L92-L98) * Use `winnow::Result` over `ModalResult` when `cut_err` isn't used * Swap `ErrMode::from_error_kind` to `ParserError::from_input` (L89) References: winnow-rs/winnow#618 References: winnow-rs/winnow#620 References: winnow-rs/winnow#622
I also edited some benchmark IDs to be consistent. For now, I'm fine having criterion write to the same ID. It's convenient for my CLI testing.
2 participants
The results are quite interesting. When parsing without recursive brackets, the performance is on par:
But when parsing with recursive bracketed subexpressions, the performance of the shunting yard declines dramatically:
linux x86_64 Intel(R) Core(TM) i5-10400F CPU @ 2.90GHz.
The same goes on macos M2.
Here are the corresponding flamegraphs (the parsing includes bracketed expressions):
pratt https://github.com/user-attachments/assets/9189ede4-bd31-494f-aeb6-ed42c31a4779)
I think it is related to frequent
raw_vec::allocatefollowed bydrop->deallocatewhen parsing recursive operators such as(1).