Skip to content

perf: reuse partition evaluator within manifests#3656

Open
dossett wants to merge 1 commit into
apache:mainfrom
dossett:reuse-partition-evaluator
Open

perf: reuse partition evaluator within manifests#3656
dossett wants to merge 1 commit into
apache:mainfrom
dossett:reuse-partition-evaluator

Conversation

@dossett

@dossett dossett commented Jul 14, 2026

Copy link
Copy Markdown

I used CODEX to analyze this problem and create this PR. I've reviewed the code and test and stand by them. This summary is written completely by a human (me) other than very light copy editing by an LLM.

Each manifest is able to use its own _ExpressionEvaluator and they are not reused across manifests or workers for the same manifest. This is a performance win over creating one for each file because the constructor performs the binding of the partition predicate to the partition scheme. My local py-spy showed an order of magnitude more time being spent constructing the evaluator than running the evaluation.


During local scan planning, PyIceberg evaluates each data file's partition values against a projected scan predicate. Profiling showed that manifest workers were spending most of their active time reconstructing _ExpressionEvaluator and rebinding the same predicate for every file, while the actual partition evaluation was comparatively cheap.

The existing partition evaluator callable is cached per partition spec and shared across manifest worker threads. Because _ExpressionEvaluator.eval() mutates its current partition record, a single evaluator cannot safely be stored in that shared callable.

This change keeps the immutable partition schema and projected expression cached per spec, but caches a factory instead of a mutable evaluator. The factory creates a separate, lazily initialized evaluator for each manifest task. Files within that manifest are processed sequentially and reuse the evaluator, while concurrent manifests never share mutable state.

Performance

The focused benchmark evaluates 1,000 files using a two-field partition spec and a 66-leaf projected predicate. Results are the mean of three in-process runs:

Files per manifest main This PR Result
1,000 0.855 s 0.063 s ~13.6x faster
1 0.847 s 0.859 s Effectively unchanged

The second case exercises the boundary where no evaluator reuse is possible. It shows that creating a task-local closure does not introduce a meaningful regression when every manifest contains only one file.

This benchmark includes partition evaluator construction, predicate binding, and evaluation. It excludes catalog access, manifest Avro I/O and decompression, metrics evaluation, residual planning, and downstream data reads, so it is not an end-to-end query-runtime claim.

Testing

  • Added coverage proving a manifest callable constructs one evaluator and reuses it across files.
  • Added coverage proving the immutable factory is cached once per partition spec while separate manifests receive separate evaluators.
  • Added coverage proving reused evaluators replace their mutable partition-record state between files.
  • Added focused benchmarks for both many files per manifest and one file per manifest.
  • Planner and expression-evaluator tests: 183 passed.
  • Full unit suite: 3,770 passed, 1,560 deselected.
  • All repository hooks pass, including Ruff, mypy, pydocstyle, codespell, and lockfile validation.

There are no API or result-semantics changes.

AI assistance

Codex assisted with implementation, test scaffolding, benchmark design, validation, and PR drafting.

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.

1 participant