Use Matcher.replaceAll in QueryObfuscator#10630
Conversation
Improves worst case performance of QueryObfuscator at the expense of average case performance In the QueryObfuscatorBenchmark, the no redact & simple redact cases drop-off by 4-6%, but the large redact improves 3x.
dougqh
added
tag: performance
dougqh
changed the title
| java.util.regex.Matcher matcher = pattern.matcher(query); | ||
| return matcher.replaceAll("<redacted>"); |
There was a problem hiding this comment.
nit: Nice, but how about to put a comment here that we are using JUL matcher, not Google one for performance reasons?
There was a problem hiding this comment.
That was actually a mistake, my IDE didn't pick up RE2 and auto-completed the wrong thing. Now, I need to double check everything. Ugh, that's what I get for rushing.
There was a problem hiding this comment.
I might just scrap this PR, since I now have a better solution nearly ready to go.
There was a problem hiding this comment.
Shouldn't it be
| java.util.regex.Matcher matcher = pattern.matcher(query); | |
| return matcher.replaceAll("<redacted>"); | |
| return pattern.matcher(query).replaceAll("<redacted>"); |
|
This pull request has been marked as stale because it has not had activity over the past quarter. It will be closed in 7 days if no further activity occurs. Feel free to reopen the PR if you are still working on it. |
|
THis pr don;t compile, also there's now this PR #11649 |
|
Yes, I've basically abandoned this one. I'm just going to close it. |
3 participants
What Does This Do
Changes QueryObfuscator to use Matcher.replaceAll to perform redaction
Improves worst case performance of QueryObfuscator at the expense of average case performance
Motivation
In the QueryObfuscatorBenchmark, the no redact & simple redact cases drop-off by 4-6%, but the large redact case improves 3x.
Additional Notes
In subsequent PR, I'm going to use regex matching on CharSequence that provide a view into the query string to further reduce the overhead. This PR is just a stop gap to mitigate the worst case behavior.
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.