Skip to content

optimize the trivial "RETURN $i" query - #764

Open
adsharma wants to merge 5 commits into
mainfrom
trivial_query_overhead
Open

optimize the trivial "RETURN $i" query#764
adsharma wants to merge 5 commits into
mainfrom
trivial_query_overhead

Conversation

@adsharma

@adsharma adsharma commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Profiling the following trivial query:

    for i in range(n):
        conn.execute("RETURN $i", {"i": i})

revealed many optimization opportunities.

  • Reuse physical plan in the prepared statement
  • Avoid unnecessary allocations

@adsharma
Phase 1: cache physical plan for repeated trivial queries
d7036eb 
Cache the PhysicalPlan (operator tree template) in CachedPreparedStatement
so that mapOperator recursion is skipped on subsequent executions.

Key changes:
- Added physicalPlanCache + resultSetDescriptor to CachedPreparedStatement
- Added prepareForReuse() to PhysicalOperator for resetting mutable state
- ResultCollector::prepareForReuse() recreates sharedState with fresh table
- TableFunctionCall::prepareForReuse() resets FTableScan scan position
- Added resetState() to TableFuncSharedState for scan-position reset
- LiteralExpressionEvaluator reads fresh value from ParameterExpression
  at init time instead of the frozen copy, enabling correct parameter
  propagation on reused plans
- cachePhysicalPlan flag in executeNoLock prevents overhead in queryNoLock

Performance: ~7-8% improvement for prepared-statement RETURN $i queries
on the implicit-cache path (118 vs 127 us/iter), no regression for the
no-params path.
@adsharma
adsharma force-pushed the trivial_query_overhead branch 2 times, most recently from be1f063 to 12332a6 Compare July 31, 2026 04:53
@adsharma
Phase 2: reuse FactorizedTable DataBlocks and DataChunkState
e0c7b31 
Avoid re-allocating per-iteration resources by keeping them alive:

- FactorizedTable::clear() keeps existing DataBlocks and resets their
  bookkeeping (freeSize, numTuples) instead of freeing + re-allocating.
  Saves a calloc(256KB) per FactorizedTable.clear() call.

- DataChunkState::getSingleValueDataChunkState() returns a function-static
  singleton instead of heap-allocating a new state + 8KB sel-vector buffer
  on every call. Safe because the selection-vector internals are immutable
  after construction.

- ResultCollector::prepareForReuse() calls sharedState->getTable()->clear()
  instead of constructing a fresh FactorizedTable. Combined with the
  FactorizedTable::clear() change, this keeps the allocation hot and
  eliminates the malloc/free cycle for the result table's DataBlock.
@adsharma
adsharma force-pushed the trivial_query_overhead branch 3 times, most recently from 3b4cfd8 to e0c7b31 Compare July 31, 2026 14:36
adsharma added 2 commits July 31, 2026 07:36
@adsharma
Phase 3: reuse ResultSet across executions of the same prepared state…
7115ff2 
…ment

Avoid the per-execution DataChunk/ValueVector allocation in
ProcessorTask::run by passing a pre-built ResultSet through the
ExecutionContext. The ResultSet is owned by CachedPreparedStatement
and outlives the cloned PhysicalPlan, so its DataChunks, ValueVectors,
and value buffers survive across the for-loop pattern:

    for i in range(n):
        conn.execute("RETURN $i", {"i": i})

The cached ResultSet is only used when the ProcessorTask is
single-threaded: a shared ResultSet would race between worker threads
writing to the same ValueVectors concurrently. Multi-threaded tasks
fall back to allocating a fresh ResultSet per call, preserving the
old behaviour.

The ResultSet::resetForReuse() helper clears per-execution state
(multiplicity, null mask, auxiliary buffer) so the next execution
starts from a clean slate.
@adsharma
Phase 3: thread-local ResultSet pool to skip per-execution allocation
7d19cac 
For a loop such as

    for i in range(n):
        conn.execute("RETURN $i", {"i": i})

each iteration was paying a 16KB+ calloc for the trivial-ResultSet's
ValueVector value buffer (plus a fresh NullMask), because
Sink::getResultSet() allocated a new ResultSet on every
ProcessorTask::run() call. Reuse the allocation across calls.

Ownership and thread safety
---------------------------

* The worker thread OWNS the ResultSet outright via a thread_local
  shared_ptr. No aliasing, no borrowing from a longer-lived cache.
  Lifetime is well-defined: dropped when the thread exits, or when a
  different prepared statement runs on this thread and the descriptor
  pointer no longer matches (we rebuild).

* A fresh descriptor pointer is a strong-enough cache key. The
  ResultSetDescriptor lives in the Sink, which lives in the PhysicalPlan
  built by executeNoLock. The descriptor pointer is stable across
  iterations of conn.execute() on the same prepared statement and
  changes for a different one. We always read the descriptor through
  the freshly-built Sink inside this run() call, before any thread_local
  lookup, so use-after-free on the descriptor is impossible by
  construction.

* Reset between runs is bounded: ResultSet::resetForReuse() clears
  multiplicity, null mask, and auxiliary buffers; value buffers are
  overwritten in place by the operators during executeInternal.

* The 'no descriptor' branch (e.g. OrderByMerge) keeps the old
  per-call allocation via sink->getResultSet().

* The drop on descriptor change is safe even with a live QueryResult
  in the C-API. The ResultSet is purely an inter-operator scratchpad
  (DataChunks / ValueVectors / value buffers) that the ResultCollector
  copies into its FactorizedTable inside executeInternal. The
  QueryResult returned to the user only holds a shared_ptr to that
  FactorizedTable; it never references the ResultSet. Once the merge
  into sharedState->getTable() is complete, the ResultSet is dead
  weight from the result path's point of view and can be freed
  without touching the live QueryResult.

Why TLS over an arena / pointer-bumping allocator
------------------------------------------------

The alternative is a thread-private bump allocator (bump ptr over a
slab, return a new region each call). It would also avoid the per-call
calloc, and in the hot path it would be slightly cheaper than TLS
because there is no shared_ptr indirection on the fast path and no
deserializer work for the 'descriptor matches' check. We did not take
that path for three reasons:

1. Reset cost. The arena would have to be rewindable or the descriptors
   would have to know which slab to free from. ResultSet::resetForReuse
   walks every DataChunk / ValueVector and toggles a few state bits;
   the per-iteration cost is O(numValueVectors) and stays bounded by
   the descriptor. A bump allocator resets in O(1) by moving the bump
   pointer, but it must then either keep the old slabs around (memory
   grows unbounded) or unmap them (slower than resetForReuse at the
   scale we care about, which is the trivial-query case where the
   descriptor has 1-2 ValueVectors).

2. Where the win actually lives. The hot path is the trivial
   prepared-statement loop:

       for i in range(n): conn.execute('RETURN $i', {'i': i})

   Here the descriptor is the same on every iteration and the
   number of distinct descriptors on a given thread is small (often 1).
   A TLS map keyed by descriptor pointer is a 1-entry cache; the
   hash/compare cost is negligible compared to the calloc we are
   avoiding. The arena's O(1) reset doesn't help because we would
   have already paid the allocation cost for the slab.

3. Code surface. TLS is local to ProcessorTask::run() and ResultSet;
   an arena-based design would need a new Allocator abstraction,
   integration with the MemoryManager (which is not a bump allocator),
   and a plan for what happens when the slab is full (fall back to
   malloc? allocate a fresh slab? per-descriptor? per-thread?).
   The data we are keeping alive -- DataChunk, ValueVector, value
   buffer, null mask -- is heterogeneous and the lifetimes do not
   nest cleanly, which is what makes a bump allocator awkward here.

   The TLS design pays a small price on the descriptor miss (one
   make_shared<ResultSet>), which only happens on the first call per
   (thread, descriptor) pair. For the trivial-query loop that is
   one allocation total per worker thread; the remaining n-1 calls
   reuse the slot.
@adsharma adsharma mentioned this pull request Jul 31, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@adsharma