perf: three-engine batch column access API + column-major ColumnFrame hot paths - #155
Merged
Conversation
Answers the long-standing question of why ColumnFrame shows no significant end-to-end advantage over RowFrame in pine-go: - Root cause 1 (first-principles): per-element OperatorInput.Item() interface tax (RLock + map lookup per element, 27x over an ideal hoisted column scan) flattens both storage layouts to the same order of magnitude. - Root cause 2: three ColumnFrame hot paths run row-major code over column storage (BuildInput validation, ItemWrites, Additions). - Root cause 3: typical pipeline shapes (recall/removals/reorder, small calibrated N) are inherently row-friendly. Records prototype A/B data (column-major fixes: BuildInput 53us -> 3.9us, transform-heavy e2e +30% for column), the layered solution path (column-major fixes -> batched column access API as the gate -> typed columns + arena), and updates perf-evolution-roadmap step 1 with the "typed columns need a batched access API to pay off" data point. Code changes remain an uncommitted prototype.
…access API Root-caused why column storage showed no advantage over row storage (see llmdoc/memory/reflections/column-vs-row-parity-investigation.md): the per-element OperatorInput.Item() interface tax (RLock + map lookup per element, 27x over an ideal column scan) flattened both layouts, and three ColumnFrame hot paths ran row-major code over column storage. ColumnFrame fixes (behavior-preserving, first-error priority kept byte-identical): - BuildInput strict/nullable validation: hoist column resolution out of the per-item loop (was one map lookup per item x field) - ApplyOutput item writes: cache the last-resolved column across field-major write runs - ApplyOutput additions: column-major two-pass batch append (was O(added x cols) map iterations) New batch access API: - types.ColumnReader optional interface + OperatorInput.ItemColumn: one lock + one lookup per column; zero-copy view on ColumnFrame (defaults force a copy), single-lock gather on RowFrame, per-element fallback for other FrameReader impls; window-aware for data_parallel shards; Item()-identical semantics incl. defaults - Built-in operator hot loops rewritten to ItemColumn: normalize, sort, shuffle, dedup, condition, resource_lookup, copy, observe_log, remote_pineapple, lua (common-mode gather + item-mode hoist) Benchmarks (Apple M5 Pro, 1000 items x 10 fields): - BuildInput/column 53us -> 4.9us (was 48% slower than row, now ~7x faster) - Additions/column 600us -> 352us - e2e transform-heavy 5000 items: column ~37% faster than row (3.56ms vs 5.68ms, -40% bytes, -20% allocs); was at parity before - recall/filter/sort shapes: row still ahead (inherent zero-copy additions + removals/reorder advantage), gap narrowed New tests: ItemColumn semantics (row/column/materialized/defaults/ shard-window/absent-field) + storage A/B benchmark suite.
…h access API Port of the pine-go column-store optimization (fbf2ef7) with identical semantics: - ColumnFrame.buildInput: hoist column/presence resolution out of the per-item strict/nullable validation loop; iteration stays item-major so first-error priority is byte-identical - ColumnFrame.applyOutput item writes: cache the last-resolved column across field runs - Frame.itemColumnView default method (optional batch read, null = fall back) + ColumnFrame zero-copy view + DataFrame single-lock gather - OperatorInput.itemColumn: element i identical to item(i, field), incl. item-default substitution; zero-copy when the ColumnFrame has no defaults for the field - Operator hot loops rewritten to itemColumn: TransformNormalize, ReorderSort, ReorderShuffle, MergeDedup, FilterCondition, TransformResourceLookup, TransformCopy, ObserveLog, TransformRemotePineapple, TransformByLua (common-mode gather + item-mode column hoist) Tests: new ItemColumnTest (row/column parity, defaults, materialized, window, absent field); full suite 251 tests green; checkstyle clean.
Port of the pine-go column-store optimization (fbf2ef7) with identical semantics: - Frame::item_column(field) virtual: all values of a field gathered under a single lock acquisition (vs per-element item() = lock + column lookup each). ColumnFrame walks the contiguous typed column; RowFrame gathers from per-row maps; window views translate offsets. - OperatorInput::item_column: element i identical to item(i, field), including item-default substitution for nil slots. - Operator hot loops rewritten: transform_normalize, reorder_sort, reorder_shuffle_by_salt, merge_dedup, filter_condition, transform_resource_lookup, transform_copy, observe_log, transform_remote_pineapple, transform_by_lua (common-mode gather + item-mode column hoist). Unlike pine-go/pine-java, the BuildInput validation and item-write paths already had batch shapes (validate_strict_items bitmap scan, write_item_field_locked), so only the read-side API was ported. Also: - arena.hpp: add missing <memory> include (std::align lives there; newer libc++ no longer pulls it in transitively) - test_remote_pineapple.cpp: fix build_input_from_frame returning an OperatorInput holding a pointer to a stack-local InputFieldSpec — a latent dangling-pointer bug that item_column's field-name compare turned into a reproducible SIGSEGV (caught by ASan) Tests: new test_item_column.cpp (both frames: item() parity, defaults, window-view offsets, absent field); full doctest suite 219 cases / 110212 assertions green incl. ASan build; clang-format applied.
/opt/homebrew/opt/luajit is where brew installs LuaJIT on Apple Silicon; without the hint, local cross-validate prebuild fails to configure on macOS while CI (linuxbrew/apt paths) is unaffected.
item_defaults only fire on slots that read as nil; random item generation rarely lines the two up, so the Defaulted substitution path — including OperatorInput.ItemColumn's defaults-copy branch added in fbf2ef7 — was under-exercised (coverage showed the branch at 0 hits across 60 rounds). gen_pipeline now punches present-nil holes (40% per slot) into recall_static / request items for fields that some operator declares item_defaults on. Present-nil rather than deletion: missing keys error in Nullable-mode readers, while present-nil passes through and triggers substitution exactly where a default is declared. New summary stat defaults_nil counts rounds where a defaulted field actually co-occurs with an explicit nil. 60-round smoke: defaults=20 defaults_nil=14, 0 divergence, ItemColumn defaults-copy branch now covered (verified via go build -cover + covdata).
Update stable docs after the column-store optimization landed on all three engines (fbf2ef7 / bf7ce0b / 95a3000 + fuzz dimension 16440e8): - reflections/column-vs-row-parity-investigation.md: replace the "uncommitted prototype" caveat with the landing status, add an implementation record (per-engine API shape table, shared semantic contract, verification results) and three implementation-phase lessons (spec raw-pointer lifetime in test helpers, quantify fuzz coverage instead of assuming, rule out machine load before blaming code for flaky divergence) - reference/operator-contract.md: new batch column access contract section (ItemColumn / itemColumn / item_column three-engine table, Item()-identical semantics incl. defaults, read-only/Execute-scoped view rules, operator author guidance) - decisions/perf-evolution-roadmap.md: step 1 note updated — the batch access API prerequisite for typed columns is now in place - guides/ci-quality-baseline.md: document the defaults+nil co-occurrence fuzz dimension and the defaults_nil summary stat; drop the hardcoded dimension count per the no-hardcoded-quantities convention - index.md: sync the three affected index entries
…workflow Replace the verbose always-step-one / skill-table boilerplate with: - explicit language conventions (Chinese for user-facing docs and communication, English for code comments and commit messages, plain-Mandarin wording rules with a jargon avoid-list) - the llmdoc plugin's compact workflow reminder (skill loading, subagent roles, .llmdoc-tmp semantics, /llmdoc:update prompt)
Contributor
🔍 PR 审查
三引擎批量列访问 API 实现质量高：语义与逐元素 逐项核对结论：
|
Liam0205
added a commit
that referenced
this pull request
Jul 7, 2026
… ownership) Both items were flagged as non-blocking by the review bot; verified real and fixed rather than carried as debt: - pine-go ItemColumn: treat a nil item-default as "no default" so the zero-copy view path stays available (nil-for-nil substitution is a no-op). Aligns the branch condition with pine-java's defaultVal == null check — outputs were already identical, but the copy-vs-view path divergence was an avoidable cross-engine inconsistency. - pine-cpp test_remote_pineapple: replace the function-static spec store (unbounded growth, not thread-safe) with an OwnedInput struct bundling spec + input, mirroring production ownership shape with scope-bound lifetime. Note: pre-commit hook requires bash>=4 (mapfile); system /bin/bash is 3.2 on this machine. Hook was run manually under homebrew bash 5 and passed; --no-verify used only to bypass the interpreter mismatch.
Contributor
🔍 PR 增量审查
本次增量正是回应上次审查标记的两个小问题（fix commit
|
Both review items were flagged as non-blocking by the review bot; verified real and fixed rather than carried as debt: - pine-go ItemColumn: treat a nil item-default as "no default" so the zero-copy view path stays available (nil-for-nil substitution is a no-op). Aligns the branch condition with pine-java's defaultVal == null check — outputs were already identical, but the copy-vs-view path divergence was an avoidable cross-engine inconsistency. - pine-cpp test_remote_pineapple: replace the function-static spec store (unbounded growth, not thread-safe) with an OwnedInput struct bundling spec + input, mirroring production ownership shape with scope-bound lifetime. Also fix .githooks/pre-commit for macOS: /bin/bash is 3.2 there and mapfile is bash 4+, so the hook aborted before checking anything. Replace mapfile with a while-read loop; verified the hook now passes end-to-end under system bash 3.2.
Liam0205
force-pushed
the
feat/column-store-batch-access
branch
from
July 7, 2026 10:18
b62b05d to
5dc3c9e
Compare
Contributor
🔍 PR 增量审查
上次审查截止点
|
This was referenced Jul 7, 2026
Liam0205
added a commit
that referenced
this pull request
Jul 7, 2026
Port pine-cpp's Column hierarchy (include/pine/column.hpp) to the other two engines: fixed-width typed columns (float64/double, string, bool) with a validity bitmap, plus a JSON fallback for mixed/composite/ present-null data. Cashes in the batch access API from PR #155 — with the per-element interface tax gone, boxing was the next bottleneck. pine-go (internal/dataframe/column.go + column_frame.go rewrite): - column interface + typedColumn[T] generics + jsonColumn fallback - construction-time type inference (one pass per field, mirrors pine-cpp make_column); present-nil disqualifies typed storage - runtime promotion on type-mismatched / nil writes (toJSON + retry), matching pine-cpp write_item_field_locked semantics - deliberate divergence from pine-cpp, documented in column.go: typed dispatch is by EXACT runtime type (float64 only for numerics) — Go's `any` preserves int vs float64 and downstream contracts observe it (%T error messages, type-prefixed dedup keys); JSON-sourced data is always float64 so the common case still gets the typed path - types.Float64ColumnReader optional interface + OperatorInput.ItemColumnFloat64: zero-copy raw []float64 window, ok only when fully present (defaults can never fire); normalize and sort hot loops use it, skipping boxing + type assertions entirely - removals/reorder move to bitmap-compaction / cycle-following inside each column (scratch shared across columns) pine-java (Column.java + ColumnFrame.java rewrite): same model — Column.build inference, DoubleColumn/StringColumn/BoolColumn/JsonColumn, promotion on set/append failure, Frame.itemColumnDoubleView default method + OperatorInput.itemColumnDouble, TransformNormalize/ReorderSort fast paths. Benchmarks (Apple M5 Pro, high ambient load — same-run A/B): - pine-go New/column: 33→40 allocs but bytes 177KB→94KB (typed arrays) - Removals/column: 88KB→1KB per op, 21→1 allocs (bitmap compaction) - e2e transform-heavy 1000: column ~0.89ms vs row ~1.37ms (~35% faster, bytes -51%); 5000: column ~3.6ms vs row ~5.6ms Tests: typed inference / promotion / present-nil semantics / fast-path gating / row-vs-column result parity on both engines (Go typed_column_test.go, Java TypedColumnTest.java); full suites green (go test -race + golangci-lint 0 issues; mvn 256 tests + checkstyle); cross-validate sections 3/4/5 green (95/95 column-store parity); 120-round three-engine differential fuzz 0 divergence.
Liam0205
added a commit
that referenced
this pull request
Jul 8, 2026
…hree engines (#157) Write-side counterpart of the batch column read API (#155) and typed columns (#156): operators hand a whole float64/double column to the frame in one call instead of N per-element SetItem records. API (per engine): - pine-go: OperatorOutput.SetItemColumnFloat64(field, []float64) - pine-java: OperatorOutput.setItemColumnDouble(field, double[]) - pine-cpp: OperatorOutput::set_item_column_double(field, vector<double>) Semantics (identical across engines, pinned by tests): - Applied at stage 2b, AFTER per-element item writes — a column write to the same field deterministically wins. - Length must equal the frame's item count (whole column or nothing); mismatch error message is byte-identical across engines. - NaN/Inf batch validation produces the same first-error message as the per-element path (item[i] write: field "f": NaN/Inf ...). - Column-store frames ADOPT the array as the column's backing storage (zero-copy, all slots present); row-store frames scatter per row in one lock window. - Counts as SetItem for OperatorType.ValidateOutput; folds into the item_writes debug snapshot; data_parallel shard merges fold column writes into offset-adjusted per-element writes. transform_normalize now writes its result column via the batch API in all three engines. pine-go transform-heavy 1000 A/B (vs master): column 0.92ms → 0.70ms (-24%), 12.1k → 4.1k allocs/op (-66%), bytes -11%; row-store also drops 15.1k → didn't regress (scatter path, boxing unavoidable). Verification: three-engine unit tests green (Go all, Java 263, C++ 225/110245 assertions); cross-validate sections 3/4/5 (95/95 exec + column-store parity, 30/30 error parity, Go vs Java vs C++); differential fuzz 120 rounds seed=42 zero divergence (row 73/0, column 47/0).
Liam0205
added a commit
that referenced
this pull request
Jul 26, 2026
Selection criteria only existed in llmdoc, so downstream users had no way to discover when column mode pays off — README said "DataFrame supports two storage modes" and stopped there, and doc/ documented no root-level config at all. Adds a "Flow-level configuration" section covering storage_mode, log_prefix, debug and skip_dead_code, with the selection criteria written out: transform-dominated + large N + few structural changes favours column, recall/filter/sort or small N stays on row, plus the mechanical reason. Deliberately states no speedup multiples. The figures floating around (~30% in llmdoc vs ~37% in the PR #155 description) are two measurements of the same experiment that disagree, they come from Go microbenchmarks on Apple silicon while README's Benchmark table is Linux 2C/4G HTTP end-to-end, and #156/#157 have already moved them three times. Points at the A/B entry points instead, so readers measure their own workload. The criteria are stable; the multiples are not. Both documented commands were run before committing: the benchmarks/ module needs its own directory and the pine_bench tag, which the first draft got wrong. The Flow snippet and the compile-time rejection of invalid values were checked too. Says nothing about what happens on an invalid value, because the runtimes disagree — pine-go and pine-java fall back to row, pine-cpp falls back to column. The Apple DSL rejects it at compile time, which is the contract worth documenting; the divergence is filed separately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Answers a long-standing question — why does column storage show no significant advantage over row storage? — then fixes the root causes on all three engines.
Root causes (investigated on pine-go, recorded in llmdoc)
Item()interface tax (first-principles cause): every operator read pays RLock + column map lookup per element — 7,210ns vs 262ns for an ideal hoisted column scan (27x). This flattens both storage layouts to the same order of magnitude before columnar layout can matter.Changes
fbf2ef7): column-major fixes for the three hot paths (first-error priority kept byte-identical) +types.ColumnReaderoptional interface +OperatorInput.ItemColumn(field)batch access (zero-copy view on ColumnFrame; defaults force a copy; per-element fallback; window-aware for data_parallel shards) + 10 built-in operator hot loops rewrittenbf7ce0b): same pattern —Frame.itemColumnViewdefault method +OperatorInput.itemColumn+ same operator set95a3000):Frame::item_columnvirtual +OperatorInput::item_column+ same operator set (read side only — C++ validation/write paths already had batch shapes). Also fixes a latent dangling-pointer bug intest_remote_pineapple.cpp(stack-localInputFieldSpecoutlived byOperatorInput, surfaced as SIGSEGV by the new field-name compare, pinned via ASan) and a missing<memory>include inarena.hpp16440e8): new defaults+nil co-occurrence dimension — coverage showed theItemColumndefaults-copy branch at 0 hits before; now exercised (defaults_nilsummary stat added)d414d89): macOS homebrew LuaJIT search hints for local cross-validateoperator-contract.md, roadmap step-1 note, CLAUDE.md language conventions rewriteResults (Apple M5 Pro)
Semantics: element i of the batch API is identical to
Item(i, field)including item-default substitution; returned views are read-only and Execute-scoped (safety via DAG hazard ordering).Verification