Performance: fast-path top-level flat scalar fields in ArrayToDataColumnsConverter#32
Open
serpentblade wants to merge 2 commits into
Open
Conversation
…verter For top-level flat scalar fields (no nesting, no repetition) the existing recursive processDataRecursively() pays O(rows) PHP function-call overhead per field. On wide tabular schemas — a common shape for DB exports, CSV-to-parquet, and analytics events — this dominates the converter cost. This change adds an early return at the top of processData() that handles the gated case directly with a tight foreach: count($path) === 1 && !$df->isArray && empty($options['nullable_levels']) && empty($options['repeated_levels']) Anything more complex (lists, maps, structs, nested fields) falls through to the existing recursive path with zero behavior change. Output bytes are identical pre/post for the gated case; the same "Malformed data: null value in not-nullable field" exception is raised for the null-in-non-nullable case. Measured against a 20K rows x 100 columns intermixed-type synthetic schema (see benchmark-wide-schema.php in the follow-up commit): ArrayToDataColumnsConverter::toDataColumns: 1.450s -> 493ms (~2.9x) End-to-end ParquetDataWriter: 2.647s -> 1.763s (~1.5x) File size: 1.8 MiB (byte-identical) Real-world payoff on a 250K-row x 430-column boolean-heavy export: 180s -> 109s.
Self-contained script: 20,000 rows x 100 columns intermixing integer, bigint, float, string, boolean, timestamp, and list types. Times the converter alone and end-to-end ParquetDataWriter. Includes one list column so the existing recursive path stays exercised — useful as a no-regression check. Output is deterministic so parquet bytes are byte-stable across runs; the file size in the benchmark output can be compared pre/post patch to confirm output is unchanged.
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
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
ArrayToDataColumnsConverter::processDatadispatches every field throughprocessDataRecursively— a static method with 8 parameters and severalconditionals, designed to handle the full range of parquet's nesting,
repetition, and definition-level semantics. For nested types (lists,
maps, structs) this is necessary. For top-level flat scalar fields —
which dominate most real-world tabular schemas (DB dumps,
CSV→parquet, analytics events) — the recursive machinery is
~10× more expensive than a tight
foreachover the row buffer.This PR adds a fast path at the top of
processDatathat handles flatscalar fields directly and falls through to the existing recursive code
for anything more complex. Output is byte-identical; the change is
purely additive.
Motivation
Measured against three production tabular exports of varying column-count
and fast-path fit
The wide-column case (typical for any export that pre-computes
boolean features per row) sees the largest win. Narrower tables
benefit proportionally to their fast-path fit. No table regressed
under matched cache conditions.
At the converter level alone the savings are even sharper:
The remaining wall-clock savings come from downstream effects (smaller
buffer sizes, fewer GC cycles, faster per-row-group setup).
The patch
Adds a fast path gated on a single check:
These four conditions together mean: top-level field (not inside a
struct/map/list), not declared as an array, and no list/map ancestors.
Under those conditions the inner work reduces to:
— a direct equivalent of what the recursive code computes for this
case, without the per-call overhead.
Correctness
The fast path produces output provably equivalent to the recursive
code for the gated case:
$data[]populated with$row[$colname]hasNulls=true: definition level 1 for value, 0 for nullhasNulls=false: definition levels omitted (sincemaxDefinitionLevel === 0)'Malformed data: null value in not-nullable field'for null in non-nullableTested via round-trip cases:
null/value mix: nulls preserved correctlyListField: scalar field hits fast path, list stillgoes through existing recursive code unchanged
Reproducible benchmark
A self-contained benchmark script that runs against a 20K-row × 100-column
synthetic schema intermixing integer / bigint / float / string / boolean /
timestamp / list types is included in this PR (or as a gist if preferred):
benchmark-wide-schema.php.Run it once on master, apply the patch, and run it again. On the
maintainer's machine the converter time should drop ~3× and end-to-end
write time ~1.5×; file output is byte-identical pre/post patch:
Compatibility