-
Notifications
You must be signed in to change notification settings - Fork 332
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
Poor path performance of JsonPathMatcher for Yaml ChangeKey and ChangeValue on large files #4650
Comments
Problem is also true of ChangeValue |
Ok more info, it wasnt the amount of files but rather 1 file at 1.6mb. I would still recommend the optional regex param as a way to mitigate it as it tears through the file way faster. |
Thanks for the added details here @jrgleason ! Let us know if you're indeed cleared to contribute. Ideally we improve performance of that JsonPathMatcher.matches without any additional arguments, but I'm not sure if that has been profiled and tried before: rewrite/rewrite-yaml/src/main/java/org/openrewrite/yaml/JsonPathMatcher.java Lines 84 to 103 in a126fc8
|
For context: did your large yaml file perhaps contain anchors? I see those are expanded, so they can also add to the complexity and run duration. |
What problem are you trying to solve?
I have a project with a bunch of YAML files. When I try to add a recipe that runs ChangeKey against a complex
oldKeyPath
it causes huge lags (in my case a dry run goes from about 6 mins to over 1 hour). To get around this I started creating a simple precondition that scans the whole file for a string (typically the key or another key that is more unique). I understand this has some pitfalls and also doesn't help is a 3rd party recipe uses it and doesn't upgrade so I was hoping we could come up with a better solution.Describe the solution you'd like
Maybe a parameter to the recipe that allows us to pass a search string as well to run the regex before the matcher. It should be done before the matcher is run here...
rewrite/rewrite-yaml/src/main/java/org/openrewrite/yaml/ChangeKey.java
Line 61 in 420f56d
Have you considered any alternatives or workarounds?
My work around, as I said, is to use a precondition to check the file for some text before running the matcher
Additional context
I found this issue through my company and I am working on trying to get approved to contribute. When this happens I should be able to provide an example. Till then I will try to recreate one on my own but in theory this should be true of any project with a lot of yaml files (maybe even just a lot of files).
Are you interested in contributing this feature to OpenRewrite?
Sure but need to finish clearance first
The text was updated successfully, but these errors were encountered: