cockroachdb / cockroachdb/pebble

db: consider adding IterContext

Open
#4,115 2 comments 0 reactions 1 assignee Claimed by @RaduBerinde View on GitHub
A-storage T-storage
Dominant language
Go
Stars
6k
Forks
584
Avg merge
16h 35m
Merged PRs (30d)
5

Description

The iterator stack has a host of configuration and global state. Today a lot of this configuration exists outside the `base.InternalIterator` interface. This forces iterator construction to propagate this configuration, every internal iterator in the stack to save a copy, and every internal iterator to zero this state on Close. Iterator.Close has an unfortunately high cost on workloads that use an iterator for a one-time seek (#4049) in part due to zeroing these structures.

We might reduce CPU and reduce the memory used by an iterator by propagating some of this information down the iterator stack during an operation by pointer. For example, we could define a `base.IterContext` struct:

```
type IterContext struct {
Compare Compare
Equal Equal
Split Split
LowerBound []byte
UpperBound []byte
Prefix []byte
Stats InternalIteratorStats
Context context.Context
Logger Logger
Comparer Comparer
}
```

Every iteration method of the `InternalIterator` interface could accept a `*IterContext` as the first parameter, which would point into a field of the `pebble.Iterator` struct when iterating using a top-level `pebble.Iterator`. With a quick survey, it looks like we would be able to remove:

- `mergingIter`:
- `logger` (16 bytes)
- `split` (8 bytes)
- `prefix` (24 bytes)
- `lower` (24 bytes)
- `upper` (24 bytes)
- `stats` (8 bytes)
- `levelIter`:
- `ctx` (16 bytes)
- `logger` (16 bytes)
- `comparer` (8 bytes)
- `cmp` (8 bytes)
- `split` (8 bytes)
- `lower` (24 bytes)
- `upper` (24 bytes)
- `prefix` (24 bytes)
- `singleLevelIterator`:
- `ctx` (16 bytes)
- `cmp` (8 bytes)
- `lower` (24 bytes)
- `upper` (24 bytes)
- `colblk.DataBlockIter`
- `cmp` (8 bytes)
- `split` (8 bytes)
- `colblk.IndexIter`
- `cmp` (8 bytes)
- `split` (8 bytes)
- `arenaskl.Iterator`
- `lower` (24 bytes)
- `upper` (24 bytes)

There are some tricky bits where global state diverges from an individual iterator's state. For example the `levelIter` avoids propagating bounds if a sstable falls wholly within the bounds. I think we'd still see benefit because, for example, some iterators maintain both global-level bounds and bounds to use within a more constrained iteration context.

Additionally, we might see some benefit to keeping frequently accessed fields (`cmp`, `split`) shared by improving cache locality.

Also propagating the iteration prefix through `Next` will facilitate #3794, because today leaf iterators don't retain the iteration prefix.

Jira issue: PEBBLE-289

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.