-
Notifications
You must be signed in to change notification settings - Fork 701
[xi-rope][find][question] Allocations during multiline regex search are hurting performance. #1192
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
Comments
I will post the essence of my conversation with @cmyr and @raphlinus on zulip chat. This is a known downside of the Rope data structure. By being a non-contiguous data storage it is not directly compatible with existing tools like regex engine. Regex engine operates on String, so allocation to a contiguous memory area is necessary. Possible workarounds:
|
@ngortheone Should we keep this open? It's not clear that it's really actionable for us at this point. |
@cmyr I don't think this is actionable at the moment. |
I will leave this here for reference |
@cmyr @raphlinus
I'm writing a parser for Org markup and I use xi-rope as underlying data structure.
There are numerous cases when I need to do a multilne regex search/match. Sometimes repeatedly. (Examples: poperty drawers are recognized by multiline regex)
In case of multiline regex search whole text of the rope is allocated into a temporary string.
https://github.com/xi-editor/xi-editor/blob/master/rust/rope/src/find.rs#L290
With big files and repeated multiline searches this really becomes a huge tax on performance.
I understand that this is due to the nature of the data structure at hand -the text stored in a set of chunks that are not connected to each other.
There are 2 questions I have:
Cow::from(String::from_iter(lines))
to make it available for subsequent searches. But if I have to maintain a plain string alongside rope - what is the benefit of using a rope? That leads me to the next questionI'd really appreciate your thoughts and comments.
The text was updated successfully, but these errors were encountered: