Add ByteMatch hashCode() to reduce transitions object count. #199
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It takes many hours to compute the machine complexity of this rule
on a laptop:
{
"field1": [{
"numeric": ["<=", 120.0]
}],
"field2": [{
"numeric": [">", 300.0]
}],
"field3": [{
"numeric": ["=", 60.0]
}],
"field4": [{
"numeric": ["<", 60.0]
}],
"field5": [{
"numeric": ["<=", 60.0]
}]
}
This is because numeric matchers are compiles as ranges for comparison.
The ranges create a sequence of numbers that then get added into the
byte machine however most of these transition to a small number
of state. See `ByteMachine.addRangePattern` for more details.
Given that the numbers are only transition to handful of end states,
we should have been merging the transtion within `ByteMap.updateTransitions`.
However the merge relies on transitions being comparable which
isn't the case for ByteMap.
After adding hashcode() and equals(), we are not able to dedupe and
avoid creating duplicates. This offers nominal benefit in rule matching
latency but has notable improvement for (1) rule addition / removal time,
and (2) when comparing the machine size / complexity.
|
Comparison from previous run https://github.com/aws/event-ruler/actions/runs/11061466520/job/30734130939#step:5:100 : |
|
As often happens, Quamina is seeing action in the same space. I could never figure out a good way to hash states and dedupe, so there is memory wastage. However ehiggins0 has an approach where he builds a trie and then uses the trie path (essentially, how you got to a state) to compute what seem to be really good-quality hashes and is getting rather good memory savings. Not merged or even PR'ed yet but apparently in production in his fork. Not sure whether the technique is applicable to Ruler but it's interesting that we're both looking at the same problems. See timbray/quamina#363 |
|
interesting indeed. I'll watch timbray/quamina#363 and Quanmina PRs for progress, the trie-based approach definitely is intriguing. |
scabraha
approved these changes
Nov 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of changes:
It takes many hours to compute the machine complexity of this rule on a laptop:
This is because numeric matchers are compiles as ranges for comparison. The ranges create a sequence of numbers that then get added into the byte machine however most of these transition to a small number of state. See
ByteMachine.addRangePatternfor more details.Given that the numbers are only transition to handful of end states, we should have been merging the transtion within
ByteMap.updateTransitions. However the merge relies on transitions being comparable which isn't the case for ByteMap.After adding hashcode() and equals(), we are not able to dedupe and avoid creating duplicates. This offers nominal benefit in rule matching latency but has notable improvement for (1) rule addition / removal time, and (2) when comparing the machine size / complexity.
Benchmark / Performance (for source code changes):
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.