perf(cpp): decode each time page once and skip unbounded time filters by ColinLeeo · Pull Request #1093 · apache/tsfile

ColinLeeo · 2026-11-12T08:19:53Z

Problem

Three related costs in AlignedChunkReader (aligned chunks; pages of single-page chunks carry no page statistics):

build_page_plan() decodes a page's timestamps to classify it, then decode_page_lazy() decoded the same page again — a redundant file read plus decompress and decode.
When the time predicate accepts every timestamp — TimeBetween over the full int64 range, which is what a query without a time bound constructs — the planner still evaluated the predicate row by row.
With the parallel page plan (the default whenever a multi-column query has several value columns), the workers decoded every time page again, so (1) did not help in that configuration.

Change

Keep the timestamps the planner decoded and hand them to the consumer: decode_page_lazy() skips non-empty slots, and the parallel page-plan workers no longer re-decode a slot the planner filled.
Add Filter::is_unbounded_time_filter() (default false, overridden by TimeBetween). Without a FIELD filter the planner marks the page FULL_PASS, so the read loop skips the per-row predicate and takes the bulk-copy path; OFFSET is consumed by the existing FULL_PASS handling. With a FIELD filter the page stays a BOUNDARY page (rows depend on the mask) and OFFSET is applied after the mask by the block reader.

Benchmark

3.2M rows / 1000 devices, table-model full scan, single thread (parallel read disabled), alternating dylib A/B. The workload only reaches the planner for chunks whose pages lack statistics, which is the case targeted here:

	read
before	0.069 s
after	0.064 s (−6%)

The measurement is from before the follow-up commits that completed the fast path (row-loop skip moved into FULL_PASS, parallel re-decode removed).

Testing

Full C++ suite: 1463 tests, 1460 passed, 3 skipped. New cases cover full-range OFFSET (parallel and serial readers), prepared multi-column queries with the pool on and off, NOT BETWEEN over the full range, and the is_unbounded_time_filter() predicate itself.

Two related fixes on the aligned chunk reader: * build_page_plan() decodes a page's timestamps to classify it, and decode_page_lazy() then decoded the same page again. For single-page chunks the times kept by the planner are exactly what the lazy path needs, so hand them over instead of reading and decoding the page twice. * When the time predicate accepts every timestamp (TimeBetween over the full int64 range, which is what a query without a time bound uses), the planner no longer evaluates the predicate row by row; it knows the whole page matches and only has to apply OFFSET. Filter gains is_unbounded_time_filter(), false by default and overridden by TimeBetween. 3.2M-row scan: 0.069s -> 0.064s (~6%).

Add regression tests for the chunk-reader changes: * UnboundedTimeFilterReturnsAllRows drives the table-reader path with a TimeBetween over the full int64 range, checking that every row comes back and that OFFSET/LIMIT still apply, in serial and parallel mode. * MultiPreparedUnboundedTimeFilterWithOffset uses the prepared multi-column path, which pushes OFFSET into the page plan; the written chunks have a single statistic-less page, so the planner reaches the BOUNDARY branch where the unbounded shortcut replaces the per-row time predicate. It also covers a bounded window and an out-of-range query.

codecov-commenter · 2026-11-12T09:37:00Z

Codecov Report

❌ Patch coverage is 80.76923% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.63%. Comparing base (fdf3808) to head (eeeadac).
⚠️ Report is 5 commits behind head on develop.

Files with missing lines	Patch %	Lines
cpp/src/reader/aligned_chunk_reader.cc	77.27%	0 Missing and 5 partials ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1093      +/-   ##
===========================================
+ Coverage    66.59%   66.63%   +0.04%     
===========================================
  Files          782      782              
  Lines        56850    56862      +12     
  Branches      9170     9173       +3     
===========================================
+ Hits         37859    37890      +31     
+ Misses       17318    17296      -22     
- Partials      1673     1676       +3

☔ 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.

Review follow-ups on the page-plan work: * An unbounded time predicate made a page a BOUNDARY page, so the read loop still called satisfy_start_end_time() once per row and lost the bulk-copy path. Without a FIELD filter such a page is now marked FULL_PASS and OFFSET is consumed by the shared FULL_PASS handling; the duplicated OFFSET logic in the unbounded branch is gone. With a FIELD filter the page stays a BOUNDARY page (the rows depend on the mask, and OFFSET is applied after the mask by the block reader). * The parallel page-plan workers re-decoded every time page, undoing the planner's reuse exactly in the default (parallel) configuration. Skip slots the planner already filled. Tests: * TimeBetween::is_unbounded_time_filter() only for a non-negated full-range TimeBetween (negated/inverted/half-open/narrow/TimeIn are false). * NOT BETWEEN over the full range returns no rows. * The prepared multi-column test now runs its whole matrix with the parallel path on and off; with the pool off it reaches decode_page_lazy(), where the planner's times are handed over (probe: every lazy decode used prefilled times), and with the pool on every worker slot is served from the planner's times. * Guard the new collect lambdas against a null result set.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues

std::numeric_limits is used in this test, but this file’s diff does not add #include <limits>.… · New
Using per_page_times_[page_idx].empty() as the 'already decoded' sentinel can cause redundant… · New
std::numeric_limits is referenced in this header, but the diff does not show #include <limits>… · New

What changed in this PR

Improves AlignedChunkReader performance by reusing decoded time pages and skipping per-row evaluation for unbounded time predicates, with new tests validating correctness across serial/parallel and prepared-query paths.

Changes:

Reuse time-page decoding results between planning and page decoding (including parallel workers).
Add Filter::is_unbounded_time_filter() with TimeBetween full-range detection to bypass per-row checks.
Add/extend tests covering unbounded time filters, OFFSET behavior, prepared multi-column queries, and negated full-range filters.

File	Description
cpp/test/reader/table_view/tsfile_reader_table_test.cc	Adds integration tests for full-range `TimeBetween` behavior with OFFSET/limits and multi-column aligned reader paths.
cpp/test/reader/prepared_series_test.cc	Adds prepared multi-column query tests validating unbounded time shortcut and OFFSET under parallel/non-parallel reads.
cpp/test/reader/filter/time_in_filter_test.cc	Adds unit test for `is_unbounded_time_filter()` semantics across filter variants.
cpp/src/reader/filter/time_operator.h	Implements `TimeBetween::is_unbounded_time_filter()` for full-range, non-negated between.
cpp/src/reader/filter/filter.h	Adds default `Filter::is_unbounded_time_filter()` API.
cpp/src/reader/aligned_chunk_reader.cc	Reuses decoded times from the planner, skips redundant decoding in parallel workers, and applies unbounded-time shortcuts.

_{Review effort: Lite}

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

6 open issues

std::numeric_limits is used in this test, but this file’s diff does not add #include <limits>.…
TimeBetween now uses std::numeric_limits, but this header should include <limits> directly to… · New
This test introduces std::numeric_limits usage; ensure this translation unit includes <limits>… · New
Dereferencing reinterpret_cast<const int64_t*>(timestamp) / `reinterpret_cast<const… · New
std::numeric_limits is referenced in this header, but the diff does not show #include <limits>…
Using per_page_times_[page_idx].empty() as the 'already decoded' sentinel can cause redundant…

_{Review effort: Lite}

Avoid reinterpret_cast to int64_t/double on the row buffer, which has no alignment guarantee; memcpy into aligned locals.

Parallel CTest processes can seed the name generator in the same clock tick; one then removes the file another is opening (E_ALREADY_EXIST at the fixture's open, observed as SEGFAULT / test failure on CI). Append pid and a per-process counter, as the writer tests already do.

develop's contiguous-time-filter helper and this branch's unbounded filter check are independent; keep both virtuals.

Copilot

🟢 Approval recommended

No active review issues block approval of the aligned-chunk read performance improvements.

No open issues 🎉

6 resolved since last review

std::numeric_limits is used in this test, but this file’s diff does not add #include <limits>.…
Dereferencing reinterpret_cast<const int64_t*>(timestamp) / `reinterpret_cast<const…
This test introduces std::numeric_limits usage; ensure this translation unit includes <limits>…
TimeBetween now uses std::numeric_limits, but this header should include <limits> directly to…
std::numeric_limits is referenced in this header, but the diff does not show #include <limits>…
Using per_page_times_[page_idx].empty() as the 'already decoded' sentinel can cause redundant…

_{Review effort: Balanced}

ColinLeeo added 3 commits November 12, 2026 16:15

style: apply spotless formatting

da8a637

ColinLeeo requested a balanced review from Copilot November 12, 2026 14:18

Copilot AI reviewed Nov 12, 2026

View reviewed changes

Comment thread cpp/test/reader/filter/time_in_filter_test.cc Resolved

Comment thread cpp/src/reader/aligned_chunk_reader.cc Resolved

Comment thread cpp/src/reader/filter/time_operator.h Resolved

ColinLeeo requested a balanced review from Copilot November 12, 2026 14:55

test(cpp): include <limits> explicitly in the time filter test

244fbb2

Copilot AI reviewed Nov 12, 2026

View reviewed changes

Comment thread cpp/src/reader/filter/time_operator.h Resolved

Comment thread cpp/test/reader/filter/time_in_filter_test.cc Resolved

Comment thread cpp/test/reader/prepared_series_test.cc Resolved

ColinLeeo added 2 commits November 12, 2026 23:17

test(cpp): copy row values out instead of casting the buffer

d024450

Avoid reinterpret_cast to int64_t/double on the row buffer, which has no alignment guarantee; memcpy into aligned locals.

Copilot stopped reviewing on behalf of ColinLeeo due to an error November 12, 2026 17:41
The job was not acquired by a runner even after multiple attempts.

Copilot stopped reviewing on behalf of ColinLeeo due to an error November 12, 2026 17:50
The job was not acquired by a runner even after multiple attempts.

merge: resolve filter.h conflict with develop

eeeadac

develop's contiguous-time-filter helper and this branch's unbounded filter check are independent; keep both virtuals.

ColinLeeo requested a balanced review from Copilot November 13, 2026 02:36

Copilot started reviewing on behalf of ColinLeeo November 13, 2026 02:37

View session

Copilot AI reviewed Nov 13, 2026

View reviewed changes

ColinLeeo merged commit b16df6b into develop Nov 13, 2026
50 checks passed

Navigation Menu

Repository navigation

perf(cpp): decode each time page once and skip unbounded time filters - #1093

perf(cpp): decode each time page once and skip unbounded time filters#1093
ColinLeeo merged 8 commits into
developfrom
perf/time-filter-and-page-reuse

ColinLeeo commented Nov 12, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Nov 12, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ColinLeeo commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Change

Benchmark

Testing

Uh oh!

codecov-commenter commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ColinLeeo commented Nov 12, 2026 •

edited

Loading

codecov-commenter commented Nov 12, 2026 •

edited

Loading