Skip to content

feat: add token frequencies - #438

Open
cheb0 wants to merge 5 commits into
mainfrom
0-doc-freqs
Open

feat: add token frequencies#438
cheb0 wants to merge 5 commits into
mainfrom
0-doc-freqs

Conversation

@cheb0

@cheb0 cheb0 commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Description

Adds token frequency which can be used to optimize query performance


  • I have read and followed all requirements in CONTRIBUTING.md;
  • I used LLM/AI assistance to make this pull request;

@dkharms
dkharms force-pushed the 336-merge-source branch 3 times, most recently from 50256a3 to b017111 Compare June 26, 2026 08:54
@dkharms
dkharms force-pushed the 336-merge-source branch 3 times, most recently from 8917131 to 261846d Compare July 6, 2026 12:37
Base automatically changed from 336-merge-source to main July 6, 2026 12:40
@cheb0
cheb0 force-pushed the 0-doc-freqs branch 3 times, most recently from a8feb9e to 323d2ff Compare July 8, 2026 08:15
@codecov-commenter

codecov-commenter commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.42857% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.05%. Comparing base (9506076) to head (f157ebc).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
frac/sealed/token/block_loader.go 83.90% 7 Missing and 7 partials ⚠️
cmd/seq-db/seq-db.go 0.00% 10 Missing ⚠️
indexwriter/blocks.go 75.00% 1 Missing and 1 partial ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🔴 Performance Degradation

Some benchmarks have degraded compared to the previous run.
Click on Show table button to see full list of degraded benchmarks.

Show table
Name Previous Current Ratio Verdict
Sealing_NoSort-4 25381c 6afc04
5314.00 allocs/op 6000.00 allocs/op 1.13 🔴
Sealing_WithSort-4 25381c 6afc04
5370.00 allocs/op 6074.00 allocs/op 1.13 🔴

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🔴 Performance Degradation

Some benchmarks have degraded compared to the previous run.
Click on Show table button to see full list of degraded benchmarks.

Show table
Name Previous Current Ratio Verdict
Sealing_NoSort-4 25381c 1d308e
5314.00 allocs/op 5977.00 allocs/op 1.12 🔴
Sealing_WithSort-4 25381c 1d308e
5370.00 allocs/op 6044.00 allocs/op 1.13 🔴

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🔴 Performance Degradation

Some benchmarks have degraded compared to the previous run.
Click on Show table button to see full list of degraded benchmarks.

Show table
Name Previous Current Ratio Verdict
AggDeep/size=1000000-4 25381c 8e734e
4659647.00 ns/op 5193054.00 ns/op 1.11 🔴
Not/size=10000-4 25381c 8e734e
4.51 ns/op 5.02 ns/op 1.11 🔴
Not/size=1000000-4 25381c 8e734e
4.66 ns/op 5.22 ns/op 1.12 🔴
ParseESTime/es_stdlib-4 25381c 8e734e
182.30 ns/op 204.10 ns/op 1.12 🔴
ParseESTime/handwritten-4 25381c 8e734e
39.96 ns/op 44.60 ns/op 1.12 🔴
ParseESTime/rfc3339-4 25381c 8e734e
51.37 ns/op 57.38 ns/op 1.12 🔴

@eguguchkin eguguchkin added this to the v0.75.0 milestone Jul 13, 2026
Comment thread config/config.go Outdated
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"`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread consts/consts.go Outdated

DefaultBulkRequestsLimit = 32
DefaultSearchRequestsLimit = 32
DefaultTokenFreqThreshold = 50

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I haven't checked what the difference would be between, say, 50 and 5.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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? :)

Comment thread frac/sealed/token/block_loader_test.go Outdated
Comment thread packer/delta_bitpacker.go
}

// 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 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe we should store it as uint32 in unpacked form after all, so we don't have this function with extra allocations.

Comment thread frac/sealed/token/block_loader.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants