Skip to content

fix: index overestimates the posting list size #5327

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BubbleCal merged 2 commits into from
Nov 24, 2025
Merged

fix: index overestimates the posting list size #5327

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df5fcf1
fix: index overestimates the posting list size
BubbleCal Nov 24, 2025
2c8673a
more
BubbleCal Nov 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions rust/lance-index/src/scalar/inverted/index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1451,10 +1451,6 @@ impl PostingListReader {
pub struct Positions(ListArray);

impl DeepSizeOf for Positions {
fn deep_size_of(&self) -> usize {
self.0.get_array_memory_size()
}

fn deep_size_of_children(&self, _context: &mut deepsize::Context) -> usize {
self.0.get_buffer_memory_size()
}
Expand Down Expand Up @@ -1619,7 +1615,7 @@ impl DeepSizeOf for PlainPostingList {
+ self
.positions
.as_ref()
.map(|positions| positions.get_array_memory_size())
.map(|positions| positions.get_buffer_memory_size())
.unwrap_or(0)
}
}
Expand Down Expand Up @@ -1716,11 +1712,11 @@ pub struct CompressedPostingList {

impl DeepSizeOf for CompressedPostingList {
fn deep_size_of_children(&self, _context: &mut deepsize::Context) -> usize {
self.blocks.get_array_memory_size()
self.blocks.get_buffer_memory_size()
+ self
.positions
.as_ref()
.map(|positions| positions.get_array_memory_size())
.map(|positions| positions.get_buffer_memory_size())
Comment on lines +1715 to +1719
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Nov 24, 2025

Choose a reason for hiding this comment

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

P1 Badge Include compressed position buffers in deep size

Switching positions to get_buffer_memory_size() means the deep size now counts only the outer list’s offset/null buffers and omits the buffers of the inner LargeBinaryArray that hold the compressed positions themselves. Because CompressedPostingList has no other deep-size traversal, any posting list with positions will have most of its memory (the compressed position payloads) ignored when the cache computes weights, so cache limits can be exceeded whenever positions are present.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

@westonpace westonpace Nov 24, 2025

Choose a reason for hiding this comment

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

Codex is wrong here. I agree with you that get_buffer_memory_size is correct. The array structure itself will already be included.

.unwrap_or(0)
}
}
Expand Down