-
Notifications
You must be signed in to change notification settings - Fork 450
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
1.9 memory usage: globset
-generated RegexSet allocates and retains 48× more memory (600MB) vs regex 1.8
#1059
Labels
Comments
BurntSushi
added a commit
to BurntSushi/ripgrep
that referenced
this issue
Aug 4, 2023
We currently implement globs by converting them to regexes, and in doing so, sometimes use grouping. In all but one case, we used non-capturing groups. But for alternations, we used capturing groups, which was likely just an oversight. We don't make use of capture groups at all, and while they usually don't have any overhead, they lead to weird cases like this one: rust-lang/regex#1059 That particular issue is also a bug in the regex crate itself, which is fixed in rust-lang/regex#1062. Note though that the bug fix in the regex crate is required. Even with this patch to globset, memory usage is reduced (by about half in rust-lang/regex#1059) but is not returned to where it was prior to the regex 1.9 release.
BurntSushi
added a commit
to BurntSushi/ripgrep
that referenced
this issue
Aug 5, 2023
We currently implement globs by converting them to regexes, and in doing so, sometimes use grouping. In all but one case, we used non-capturing groups. But for alternations, we used capturing groups, which was likely just an oversight. We don't make use of capture groups at all, and while they usually don't have any overhead, they lead to weird cases like this one: rust-lang/regex#1059 That particular issue is also a bug in the regex crate itself, which is fixed in rust-lang/regex#1062. Note though that the bug fix in the regex crate is required. Even with this patch to globset, memory usage is reduced (by about half in rust-lang/regex#1059) but is not returned to where it was prior to the regex 1.9 release.
BurntSushi
added a commit
that referenced
this issue
Aug 5, 2023
I originally prided myself on not having a dedicated `is_match` routine on the meta regex engine's internal `Strategy` trait, and actually spent a fair amount of attention ensuring that `is_match` and `find` always returned the same results. That is, `is_match` returns true if and only if `find` returns a match. But the fix in the previous commits for #1059 means that a `PikeVM` and a `BoundedBacktracker` can be used to run a search with an NFA that has no capture states. Since both engines are implemented to only track offsets via those capture states, it follows that the only thing that can be returned in such cases is whether a match occurs (and if so, which pattern matched). That in turn means that `is_match` can return `true` while `find` can return `None` for the same search. This is because the latter returns `None` even when a match is found but there are no capture states to record the offsets of the match. This in theory could be resolved by adding APIs to the `PikeVM` and the `BoundedBacktracker` that return a `HalfMatch` without depending on any capture states at all. Then `is_match` could be implemented in terms of those APIs. That is probably the right path, but it's pretty gnarly to do without breaking changes and I don't want to do any breaking changes right now. So instead, we just add a special path to the meta regex engine for `is_match` and permit some cases to have different results between `is_match` and `find`. Sigh.
BurntSushi
added a commit
that referenced
this issue
Aug 5, 2023
I originally prided myself on not having a dedicated `is_match` routine on the meta regex engine's internal `Strategy` trait, and actually spent a fair amount of attention ensuring that `is_match` and `find` always returned the same results. That is, `is_match` returns true if and only if `find` returns a match. But the fix in the previous commits for #1059 means that a `PikeVM` and a `BoundedBacktracker` can be used to run a search with an NFA that has no capture states. Since both engines are implemented to only track offsets via those capture states, it follows that the only thing that can be returned in such cases is whether a match occurs (and if so, which pattern matched). That in turn means that `is_match` can return `true` while `find` can return `None` for the same search. This is because the latter returns `None` even when a match is found but there are no capture states to record the offsets of the match. This in theory could be resolved by adding APIs to the `PikeVM` and the `BoundedBacktracker` that return a `HalfMatch` without depending on any capture states at all. Then `is_match` could be implemented in terms of those APIs. That is probably the right path, but it's pretty gnarly to do without breaking changes and I don't want to do any breaking changes right now. So instead, we just add a special path to the meta regex engine for `is_match` and permit some cases to have different results between `is_match` and `find`. Sigh.
This is fixed on crates.io in |
Thank you greatly. I confirmed that Buck2 memory usage with regex 1.9.2 and globset 0.4.13 is at or slightly below what we had prior to the regex 1.9 update. |
facebook-github-bot
pushed a commit
to facebook/errpy
that referenced
this issue
Aug 5, 2023
Summary: BurntSushi has fixed a memory usage regression introduced by regex 1.9 which caused Buck to allocate and retain significantly more memory when using moderately sized `buck2_common::ignores::ignore_set::IgnoreSet` objects concurrently from many threads. - Bug report: **[rust-lang/regex#1059](rust-lang/regex#1059) *"1.9 memory usage: globset-generated RegexSet allocates and retains 48× more memory (600MB) vs regex 1.8"*** - Globset fix: **[BurntSushi/ripgrep#25770](https://github.com/BurntSushi/ripgrep/pull/25770) *"globset: use non-capture groups in regex transform"*** - Regex fix: **[rust-lang/regex#1062](rust-lang/regex#1062) *"fix memory usage regression for RegexSet with capture groups"*** Reviewed By: zertosh Differential Revision: D48095372 fbshipit-source-id: ec11c2bcaccbd26354d6d0a0398000134eaf3681
facebook-github-bot
pushed a commit
to facebook/sapling
that referenced
this issue
Aug 5, 2023
Summary: BurntSushi has fixed a memory usage regression introduced by regex 1.9 which caused Buck to allocate and retain significantly more memory when using moderately sized `buck2_common::ignores::ignore_set::IgnoreSet` objects concurrently from many threads. - Bug report: **[rust-lang/regex#1059](rust-lang/regex#1059) *"1.9 memory usage: globset-generated RegexSet allocates and retains 48× more memory (600MB) vs regex 1.8"*** - Globset fix: **[BurntSushi/ripgrep#25770](https://github.com/BurntSushi/ripgrep/pull/25770) *"globset: use non-capture groups in regex transform"*** - Regex fix: **[rust-lang/regex#1062](rust-lang/regex#1062) *"fix memory usage regression for RegexSet with capture groups"*** Reviewed By: zertosh Differential Revision: D48095372 fbshipit-source-id: ec11c2bcaccbd26354d6d0a0398000134eaf3681
facebook-github-bot
pushed a commit
to facebook/hhvm
that referenced
this issue
Aug 5, 2023
Summary: BurntSushi has fixed a memory usage regression introduced by regex 1.9 which caused Buck to allocate and retain significantly more memory when using moderately sized `buck2_common::ignores::ignore_set::IgnoreSet` objects concurrently from many threads. - Bug report: **[rust-lang/regex#1059](rust-lang/regex#1059) *"1.9 memory usage: globset-generated RegexSet allocates and retains 48× more memory (600MB) vs regex 1.8"*** - Globset fix: **[BurntSushi/ripgrep#25770](https://github.com/BurntSushi/ripgrep/pull/25770) *"globset: use non-capture groups in regex transform"*** - Regex fix: **[rust-lang/regex#1062](rust-lang/regex#1062) *"fix memory usage regression for RegexSet with capture groups"*** Reviewed By: zertosh Differential Revision: D48095372 fbshipit-source-id: ec11c2bcaccbd26354d6d0a0398000134eaf3681
facebook-github-bot
pushed a commit
to facebookincubator/below
that referenced
this issue
Aug 5, 2023
Summary: BurntSushi has fixed a memory usage regression introduced by regex 1.9 which caused Buck to allocate and retain significantly more memory when using moderately sized `buck2_common::ignores::ignore_set::IgnoreSet` objects concurrently from many threads. - Bug report: **[rust-lang/regex#1059](rust-lang/regex#1059) *"1.9 memory usage: globset-generated RegexSet allocates and retains 48× more memory (600MB) vs regex 1.8"*** - Globset fix: **[BurntSushi/ripgrep#25770](https://github.com/BurntSushi/ripgrep/pull/25770) *"globset: use non-capture groups in regex transform"*** - Regex fix: **[rust-lang/regex#1062](rust-lang/regex#1062) *"fix memory usage regression for RegexSet with capture groups"*** Reviewed By: zertosh Differential Revision: D48095372 fbshipit-source-id: ec11c2bcaccbd26354d6d0a0398000134eaf3681
facebook-github-bot
pushed a commit
to facebookexperimental/rust-shed
that referenced
this issue
Aug 5, 2023
Summary: BurntSushi has fixed a memory usage regression introduced by regex 1.9 which caused Buck to allocate and retain significantly more memory when using moderately sized `buck2_common::ignores::ignore_set::IgnoreSet` objects concurrently from many threads. - Bug report: **[rust-lang/regex#1059](rust-lang/regex#1059) *"1.9 memory usage: globset-generated RegexSet allocates and retains 48× more memory (600MB) vs regex 1.8"*** - Globset fix: **[BurntSushi/ripgrep#25770](https://github.com/BurntSushi/ripgrep/pull/25770) *"globset: use non-capture groups in regex transform"*** - Regex fix: **[rust-lang/regex#1062](rust-lang/regex#1062) *"fix memory usage regression for RegexSet with capture groups"*** Reviewed By: zertosh Differential Revision: D48095372 fbshipit-source-id: ec11c2bcaccbd26354d6d0a0398000134eaf3681
facebook-github-bot
pushed a commit
to facebook/ocamlrep
that referenced
this issue
Aug 5, 2023
Summary: BurntSushi has fixed a memory usage regression introduced by regex 1.9 which caused Buck to allocate and retain significantly more memory when using moderately sized `buck2_common::ignores::ignore_set::IgnoreSet` objects concurrently from many threads. - Bug report: **[rust-lang/regex#1059](rust-lang/regex#1059) *"1.9 memory usage: globset-generated RegexSet allocates and retains 48× more memory (600MB) vs regex 1.8"*** - Globset fix: **[BurntSushi/ripgrep#25770](https://github.com/BurntSushi/ripgrep/pull/25770) *"globset: use non-capture groups in regex transform"*** - Regex fix: **[rust-lang/regex#1062](rust-lang/regex#1062) *"fix memory usage regression for RegexSet with capture groups"*** Reviewed By: zertosh Differential Revision: D48095372 fbshipit-source-id: ec11c2bcaccbd26354d6d0a0398000134eaf3681
facebook-github-bot
pushed a commit
to facebookincubator/reindeer
that referenced
this issue
Aug 5, 2023
Summary: BurntSushi has fixed a memory usage regression introduced by regex 1.9 which caused Buck to allocate and retain significantly more memory when using moderately sized `buck2_common::ignores::ignore_set::IgnoreSet` objects concurrently from many threads. - Bug report: **[rust-lang/regex#1059](rust-lang/regex#1059) *"1.9 memory usage: globset-generated RegexSet allocates and retains 48× more memory (600MB) vs regex 1.8"*** - Globset fix: **[BurntSushi/ripgrep#25770](https://github.com/BurntSushi/ripgrep/pull/25770) *"globset: use non-capture groups in regex transform"*** - Regex fix: **[rust-lang/regex#1062](rust-lang/regex#1062) *"fix memory usage regression for RegexSet with capture groups"*** Reviewed By: zertosh Differential Revision: D48095372 fbshipit-source-id: ec11c2bcaccbd26354d6d0a0398000134eaf3681
facebook-github-bot
pushed a commit
to facebookexperimental/hermit
that referenced
this issue
Aug 5, 2023
Summary: BurntSushi has fixed a memory usage regression introduced by regex 1.9 which caused Buck to allocate and retain significantly more memory when using moderately sized `buck2_common::ignores::ignore_set::IgnoreSet` objects concurrently from many threads. - Bug report: **[rust-lang/regex#1059](rust-lang/regex#1059) *"1.9 memory usage: globset-generated RegexSet allocates and retains 48× more memory (600MB) vs regex 1.8"*** - Globset fix: **[BurntSushi/ripgrep#25770](https://github.com/BurntSushi/ripgrep/pull/25770) *"globset: use non-capture groups in regex transform"*** - Regex fix: **[rust-lang/regex#1062](rust-lang/regex#1062) *"fix memory usage regression for RegexSet with capture groups"*** Reviewed By: zertosh Differential Revision: D48095372 fbshipit-source-id: ec11c2bcaccbd26354d6d0a0398000134eaf3681
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What version of regex are you using?
1.9.1
Describe the bug at a high level.
I am investigating a recent large memory usage regression in Buck2 that bisected to a
regex
update from 1.8.4 to 1.9.0, and persists in 1.9.1.There are some long-lived
GlobSet
objects in Buck2 corresponding to filepath patterns that buck has been configured to ignore. The real-world code is here: https://github.com/facebook/buck2/blob/4d300b08ec0c742952f4cdd9abf1c1055c7a75ab/app/buck2_common/src/ignores/ignore_set.rs.I instrumented the
RegexSet
created by the globset crate, and was able to create the following repro that shows +600MB being allocated and retained by theRegexSet
implementation for a seemingly pretty reasonably sized globset.What are the steps to reproduce the behavior?
cargo run --release
:What is the actual behavior?
Regex 1.8.4:
Regex 1.9.1:
What is the expected behavior?
I am interested in knowing whether this amount of allocation is intended for the above regex, or if this might be fixable.
Buck2 is not particularly sensitive to the total amount of allocation done, but very sensitive to steady-state retained memory (with widely used RegexSet objects sitting around) and high-water mark.
The text was updated successfully, but these errors were encountered: