feat: Add new Drain tokenizer that splits on most punctuation #13143
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
cyriltovena
reviewed
Jun 6, 2024
| @@ -0,0 +1 @@ | |||
| package output | |||
cyriltovena
reviewed
Jun 6, 2024
| @@ -139,7 +141,7 @@ func DefaultConfig() *Config { | |||
| // MaxClusterDepth and SimTh, the less the chance that there will be | |||
| // "similar" clusters, but the greater the footprint. | |||
| SimTh: 0.3, | |||
| MaxChildren: 100, | |||
| MaxChildren: 15, | |||
cyriltovena
reviewed
Jun 6, 2024
|
|
||
| type LineTokenizer interface { | ||
| Tokenize(line string) []string | ||
| Join(tokens []string) string | ||
| Tokenize(line string) ([]string, interface{}) |
There was a problem hiding this comment.
I wonder if generics would work here, just a thought. I know interface have a cost when casting for instance.
cyriltovena
reviewed
Jun 6, 2024
|
|
||
| func (p *punctuationTokenizer) Tokenize(line string) ([]string, interface{}) { | ||
| tokens := make([]string, len(line)) // Maximum size is every character is punctuation | ||
| spacesAfter := make([]int, strings.Count(line, " ")) // Could be a bitmap, but it's not worth it for a few bytes. |
There was a problem hiding this comment.
You might want to use a pool for this one. Prometheus has a good sync.Pool that works in buckets.
This was referenced Jun 10, 2024
This was referenced Jul 3, 2024
Closed
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.
What this PR does / why we need it:
-characters are treated as part of a single token.Perf wise, this PR is ~50% higher CPU usage compared to previous Drain & much less allocations (so hopefully less GC). I will continue to do some perf optimizations in a separate PR to try and improve this.
Data 1:
Benchmark for using the new "punctuation" tokenizer vs the old "splitting" tokenizer:
Data 2:
Benchmark for my custom deduplicatePlaceholders vs a solution using
regexp.MustCompile("<_>+").ReplaceAllLiteralString: