Skip to content

Read each chunk's column types under one lock - #1221

Open
quinnj wants to merge 2 commits into
mainfrom
claude/one-lock-per-chunk
Open

quinnj wants to merge 2 commits into
mainfrom
claude/one-lock-per-chunk

Conversation

@quinnj

@quinnj quinnj commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Builds on #1222; merge that first. This branch contains #1222's commit: the copy below widens the window in which a chunk parses under a stale type, and without #1222 a typemap target outside the promotion order then turns a sparse column into text on every parallel read (an independent review measured 40 of 40 runs; main already does it in about 30 of 40).

directchunk! took the shared promotion lock once per column per chunk to read that column's current type and final vector. A 1,000-column file in 118 chunks took it 118,000 times per read, and that wait showed up in 8-task profiles. Each chunk now copies every column's type and final once, under one lock.

Why a stale copy is safe: if another chunk promotes a column after the copy, this chunk parses that column under the old type into the old final, records the old type in segtypes, and directwave! re-parses it into the promoted final after the wave (redirect!; a chunk that parsed as Missing only refills its slice). That is the existing path for a promotion that lands while a chunk is parsing the column; the copy widens the window in which it can happen, which can only add re-parses, and only when a promotion races with the copy. A chunk's own promotions still read the current type under the lock. The cost: a chunk whose copy predates a promotion parses that column under the old type before the re-parse and keeps the replaced final vector alive until the chunk finishes; both happen only when a promotion lands during the wave, which sampling makes rare.

Interleaved process pairs, Julia 1.12.6, M3 Max (load average about 8):

file tasks main this PR
wide_numeric (124 MB, 18k rows x 1,000 columns) 8 0.057 / 0.085 / 0.071 s 0.045 / 0.063 / 0.057 s
same, total CPU 8 0.40 / 0.52 / 0.53 s 0.30 / 0.39 / 0.38 s
wide_numeric 1 0.38 s 0.38 s
long_numeric (5M x 8), mixed (2.5M x 10) 8 within noise within noise

Review: my own pass, then an independent agent review (stale detection, the free Missing upgrade, String finals and adopted slots, limit, Timestamp widening; 144 parallel runs matching serial), which found the typemap interaction now fixed by #1222.

  • Traced every consumer of the per-chunk state: stale detection compares segtypes[k][j] with the frozen final type, so any chunk that parsed under an old type is re-parsed; String finals adopt each chunk's bytes into the chunk's own slot after the wave; Timestamp nanosecond-to-microsecond widening reads only segtypes and the source spans.
  • The snapshot is taken after the chunk's structural row checks and before any column parse; nothing between them reads promo or finals.

Verification:

  • New testset promotions that land while another chunk parses: nsample=1 seeds every column from the first row, so all 40 columns promote across 948 chunks; four parallel reads must equal the serial read in values, types, and problems, with and without a typemap. It guards the re-parse path this change leans on (the speedup itself is measured above, not tested).
  • Full suite --check-bounds=yes -t4: 331,812 passed (head, including Re-parse a stale chunk under the column's current type #1222).
  • 300 seeds of a cross-configuration consistency fuzzer (every reader, task count, and chunk size), 150 seeds of a feature fuzzer, and a promotion probe (one odd value at the first, middle, and last row of 200k, at 1 and 8 tasks): no differences.

🤖 Generated with Claude Code

When one chunk promoted a column while another was still parsing it under
an older type, the second chunk joined its detected type with the
column's current type. With a typemap target outside the promotion order
(Int64 => Int32, or the documented Timestamp{Nanosecond} => DateTime)
that join is String, so a sparse column read Int32 serially and text in
most parallel reads (29 of 40 at 2 threads, 28 of 40 at 4). The conflict
was found under the old type and says nothing against the current one,
so the chunk now re-parses under the current type. Both drivers route
through _promotemapped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
directchunk! took the shared promotion lock once per column per chunk
to read that column's type and final vector. A 1,000-column file in
118 chunks took it 118,000 times per read, and the waiting showed up in
8-task profiles. Each chunk now copies every column's type and final
once, under one lock. A column that another chunk promotes after the
copy parses under its old type and is re-parsed after the wave, which
is what already happens when the promotion lands while the chunk is
parsing that column.

wide_numeric.csv (124 MB, 18k rows x 1,000 columns), 8 tasks,
interleaved: 0.078-0.083 -> 0.061-0.062 s wall, total CPU 0.48-0.54 ->
0.38-0.39 s.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@quinnj
quinnj force-pushed the claude/one-lock-per-chunk branch from 46afcc0 to 44886f5 Compare October 22, 2026 11:28
@codecov

codecov Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.79%. Comparing base (2f84f42) to head (44886f5).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1221   +/-   ##
=======================================
  Coverage   95.79%   95.79%           
=======================================
  Files           9        9           
  Lines        5566     5570    +4     
=======================================
+ Hits         5332     5336    +4     
  Misses        234      234           

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

This branch has not been deployed

No deployments
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