internal/gogrep: use external MatcherState #286
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The reason we have to clone rule set every time we do
Engine.Run
is
gogrep.Pattern
internal state. Every gogrep matcher has mutablestate that can't be combined with the thread-safe API of
Engine.Run
.Cloning is usually not very expensive, we just copy a few dozens
of slices here and there, but it's noticeable for both small and
big rule file sizes.
If we move the mutable shared state out of the matchers and keep
it somewhere in the per-run or per-context scope, we should be
able to reuse it without races and there will be no need to
copy the matchers/patterns on their own as their internal structure
will become immutable (they receive a mutable state via param).
My experiments show ~15% speedup on the current stage.
I may move gogrep state even higher, to the per-context scope
so it's reused even more efficiently. But as a proof of concept
we can see that thread-safe version of
Engine.Run
is possiblewithout excessive copying.
With small rules file and small target file:
With big rules file and small targe file:
With small rules file and big target file size:
With huge rules file and big target file size:
Fixes #285