Skip to content
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

Transform lazy builtin regexes for finite automaton compatibility, use that for graal #228

Closed
masklinn opened this issue Oct 26, 2024 · 0 comments

Comments

@masklinn
Copy link
Contributor

As such, it would make sense to:

The one major drawback is that re2 currently uses re for its actual extraction. So here there are two options:

  • using re2 for extraction, if the back transformation leads to better performances
  • or have the finite automaton engines perform the transformation at load time internally

The latter would be costlier on loads, but it would mean less compilation woes, working with yaml data, and would make for less trouble in graal (in that it would patch the lazy callbacks with a transformation pass).

masklinn added a commit to masklinn/uap-python that referenced this issue Oct 28, 2024
As I've discovered a while ago, finite automaton engines are not very
fond of large bounded repetitions.

In re2 and regex, that mostly translates to increased memory
consumption (e.g. in their default modes, converting `.*` to
`.{0,500}` increases the pattern's size by 115x in re2 and 84x in
regex, if a capture is added on top then regex balloons to 219x),
there is a performance impact but it's high single digit to low
double, in regex at least (didn't test re2).

However as it turns out Graal uses a JIT-ed DFA, and it *really*
doesn't like these patterns, it spends a lot of time JIT-compiling
(this is apparently the source of the extra 300% CPU use I could
observe on what are purely single-threaded workloads, the JIT
desperately trying to optimise regexes) them with no gain in
performance: down-converting the regex back to the sensible increases
performances by ~25%, though it doesn't seem to impact memory use...

So... do that: `fa_simplifier` is the same idea as
ua-parser/uap-rust@29b9195 but from
the Python side, and applied to graal and re2 (not regex because it
does that internally as linked above).

Also switch Graal over to the lazy builtins, it kinda spreads the cost
but it seems stupid to compile the regexes only to immediately swap
(fa_simplifier) and recompile them... so don't do that, especially as
I couldn't be arsed to make the replacement conditional (so every
eager regex is recompiled, even though only those which actually got
modified by `fa_simplifier` need it...).

Fixes ua-parser#228
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant