improve regex validation performance.#825
Conversation
the regexes are cached in a lazy static variable to avoid reparse
on each evaluation. parse is expensive compared to eval.
added a benchmark suite to verify the change. local results:
before:
```
long regex time: [57.194 µs 57.266 µs 57.372 µs]
short regex time: [5.0102 µs 5.0126 µs 5.0150 µs]
```
after:
```
long regex time: [297.06 ns 297.50 ns 297.96 ns]
short regex time: [114.12 ns 114.41 ns 114.72 ns]
```
the toolchain bump i had to do for local development. I could not
bump the toolchain all the way to latest because increased strictness
of irrefutable-let warnings regresses several trybuild test cases.
see oxidecomputer#682
ahl
left a comment
There was a problem hiding this comment.
Can you tell me about the utility you see of checking in typify-bench? It's useful to see in this case (thank you!), but would you anticipate this being useful to continue using in some capacity? If so how would you imagine its use?
Thanks for submitting this PR; looks close.
in this case, IMO when someone commits a "performance improvement", having both the numbers observed and the test case that was exercised next to the commit are good for review, like with unit test cases. for example, if regress develops a macro that emits compile-time regexes, you can swap it in and see the result. for my own practice, i always formalize performance work like this when i'm doing it. it's just nice to have the ground broken so you can throw a test case on the pile the next time you think something might be slow. but, i'm not offended if you'd rather not maintain it, and i can prune it. the other case is if you want to set performance SLAs or guard against performance regressions; typically you'd set up benchmarks in your CI system and keep the data there and actually fail the build past a certain tolerance. obviously that would require more work. i've done that before, though less frequently (actually doing that with my current project consuming typify). lastly -- since performance improvements are highly platform-specific, you might want to pull the change down and verify the results locally as part of the review. i'm working on an Apple M1 Max. YMMV (by a lot). |
Co-authored-by: Adam Leventhal <adam.leventhal@gmail.com>
|
I appreciate the explanation. If you would please prune the test; I don't think we'd maintain it. And we'll have this PR as a record for the future if we ever want to resurrect it. Thank you! |
the regexes are cached in a lazy static variable to avoid reparse on each evaluation. parse is expensive compared to eval. added a benchmark suite to verify the change. local results:
before:
after:
the toolchain bump i had to do for local development. I could not bump the toolchain all the way to latest because increased strictness of irrefutable-let warnings regresses several trybuild test cases.
see #682