-
-
Notifications
You must be signed in to change notification settings - Fork 257
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
Improve line, col calculate performance by use move cursor on Pairs Iterator. #754
Conversation
8db068c
to
711140b
Compare
711140b
to
e052c89
Compare
c424b06
to
6b8e95a
Compare
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.
one alternative for testing may be a different test parser definition, plus check that position and pair results match ?
btw is there a need for fast-line-col
with this, or should that feature be removed? @huacnlee @NoahTheDuke
We still need There have many case use |
2e47fb7
to
370b938
Compare
ee0f1fc
to
a06a837
Compare
a06a837
to
a21627a
Compare
Hi @tomtau I have done all, please review again. |
ref: #707, #560
This implementation is based by I use in AutoCorrect: https://github.com/huacnlee/autocorrect/blob/v2.5.5/autocorrect/src/result.rs#L267
Add
line_col
method for get(line, col)
in pairIn this method, line, col has managed by move_cursor on Pairs iterator
next
method. For a large input string this method will be faster, because it does not need to re-calculate all of the input string. The line, col value has been cached in the pair and accumulate bynext
method.And I keep the old
line_col
method inPosition
struct, because I found that has been used in other withoutPairs
iterator. This method still works, but it will be slower than the new method.Benchmark
Limitation
This implementation only work for forward iteration for
Pairs
. So iterate backward will fallback toPosition::line_col
method.