refactor: extract pretok algo#2134
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
ArthurZucker
left a comment
ArthurZucker
left a comment
There was a problem hiding this comment.
feels like policy is not needed and classify alone can do this.
Otherwise makes a lot of sense.its super impressive that this just gives 8x.
Pushing to out is fine as long as out is pre allocated for worst case IMO!
| classify: impl Fn(char) -> C, | ||
| policy: impl Fn(C) -> SplitPolicy, |
There was a problem hiding this comment.
Do we want generics instead here?
There was a problem hiding this comment.
What do you have in mind?
| pub fn split<C: Copy + PartialEq>( | ||
| text: &str, | ||
| out: &mut Vec<Split>, | ||
| classify: impl Fn(char) -> C, | ||
| policy: impl Fn(C) -> SplitPolicy, | ||
| ) { |
There was a problem hiding this comment.
Could be a blanket implementation in the pipeline::PreTokenizer trait, wdyt?
There was a problem hiding this comment.
Meh, the fn knows nothing about PreTokenizer, doesn't need self so I'd rather leave it as its own separate thing.
|
|
||
| impl pipeline::PreTokenizer for Whitespace { | ||
| // XXX: surprisingly, inlining here yields 10-15% slower performance | ||
| #[inline(never)] |
There was a problem hiding this comment.
Why not letting the compiler handle this?
There was a problem hiding this comment.
We're not sure it'll pick the right choice. Here I've tested and observed that if this code is inlined, perf degrades, so we tell the compiler to make sure it always makes the right decision.
Signed-off-by: Luc Georges <luc.sydney.georges@gmail.com>
McPatate
force-pushed
the
refactor/extract_pretok_algo
branch
from
2961689 to
5fc5280
Compare
4 participants
When going through the punctuation pretokenizer implementation, I noticed that I would have to reimplement the same kind of algo for each variant of its inner
behaviorfield, which is also the same as the Whitespace and Bert algo.Extracting this into an easy to adapt API for each pretokenizer, while maintaining the perf gains shown by the other two variants.