feat: add token frequencies - #438
Conversation
50256a3 to
b017111
Compare
8917131 to
261846d
Compare
a8feb9e to
323d2ff
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #438 +/- ##
==========================================
+ Coverage 70.68% 71.05% +0.36%
==========================================
Files 233 232 -1
Lines 18399 18402 +3
==========================================
+ Hits 13005 13075 +70
+ Misses 4416 4345 -71
- Partials 978 982 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🔴 Performance DegradationSome benchmarks have degraded compared to the previous run. Show table
|
🔴 Performance DegradationSome benchmarks have degraded compared to the previous run. Show table
|
🔴 Performance DegradationSome benchmarks have degraded compared to the previous run. Show table
|
| Tokens struct { | ||
| // FreqThreshold specifies minimum number of lids (postings) a token should have | ||
| // so that frequency for that token will be stored inside token blocks. | ||
| FreqThreshold int `config:"freq_threshold" default:"50"` |
There was a problem hiding this comment.
Nit: In the future, I would suggest evolving this approach by using a percentile‑based threshold instead of a fixed one. The distribution of posting sizes may vary across different datasets. Rather than empirically tuning this threshold, I propose specifying the percentile for which the hint should be applied.
From a performance perspective, we could apply the hint to all tokens, but that would be expensive in terms of memory. Thus, this parameter would indicate how much we are willing to increase the dataset size in order to enable hinting for a subset of queries. This percentile‑based threshold can be computed incrementally in the active fraction and used during sealing.
Additionally, I suggest storing this threshold per fraction (its absolute value may differ). If for fraction A the threshold is X, this would mean that if a token lacks a hint, its posting size is ≤ X.
|
|
||
| DefaultBulkRequestsLimit = 32 | ||
| DefaultSearchRequestsLimit = 32 | ||
| DefaultTokenFreqThreshold = 50 |
There was a problem hiding this comment.
Nit: I haven't checked what the difference would be between, say, 50 and 5.
There was a problem hiding this comment.
What I meant was — have you actually checked what the dataset size difference would be between threshold 5 and threshold 50 on our real data? :)
| } | ||
|
|
||
| // CompressDeltaBitpackUint16 uses a temporary buffer to copy and cast values from uint16 to uint32 so it's a bit slower than CompressDeltaBitpackUint32. | ||
| func CompressDeltaBitpackUint16(dst []byte, values []uint16, buf []uint32) []byte { |
There was a problem hiding this comment.
nit: maybe we should store it as uint32 in unpacked form after all, so we don't have this function with extra allocations.
Description
Adds token frequency which can be used to optimize query performance