Perf/optimizations#19
Merged
Merged
Conversation
AbstractVector hot paths (add, subtract, scale, dot, distance, magnitude, normalize) all used array_map(fn) with a closure invoked per component. The closure allocation + variadic call dominated the runtime — magnitude on a Vec2 was spending 3.7us doing 2 multiplies. Two changes: 1. Rewrite all hot methods to use foreach with direct index access. Same algorithm, no closures. 2. Add bool \$trusted = false constructor flag on AbstractVector. Arithmetic results (add/sub/scale/normalize) are dimension- preserving and numeric-preserving by construction, so they skip validateComponents() via new static(\$result, true). Numbers (100k iters): Vec2 add+sub+scale: 20.17us -> 6.43us (3.1x) Vec2 dot: 3.70us -> 2.00us (1.9x) Vec2 magnitude: 3.74us -> 0.98us (3.8x) Vec3 add+sub+scale: 24.98us -> 8.44us (3.0x) Vec4 add+sub+scale: 29.48us -> 8.11us (3.6x) Behavior preserved. 592/592 tests pass.
The foreach-based hot paths on AbstractVector still pay per-component loop overhead and per-component array writes. For fixed-dimension vectors the dimension is known statically — unroll the math entirely. Override add/subtract/scale/dot/magnitude/distance/normalize on each of Vec2, Vec3, Vec4 with direct \$components[0], \$components[1], ... access. Parameter widens from self to AbstractVector (PHP LSP rules disallow narrowing) but is checked with instanceof self in each method to preserve the cross-dimension error contract. Numbers (100k iters), cumulative vs original abstract: Vec2 add+sub+scale: 20.17us -> 3.29us (6.1x) Vec2 dot: 3.70us -> 0.68us (5.4x) Vec2 magnitude: 3.74us -> 0.81us (4.6x) Vec3 add+sub+scale: 24.98us -> 3.79us (6.6x) Vec4 add+sub+scale: 29.48us -> 3.55us (8.3x) Per-op floor is now ~1.1us — instanceof check + 3-4 float ops + array literal + new self + readonly assign. The remaining cost is PHP object construction itself. Behavior preserved (same exceptions, same results). 592/592 tests pass.
… dispatch Struct construction was ~3us per field for a typical schema. Five small fixes, each individually small, compound on hot paths (Laravel form requests, batch deserialization, etc.): 1. Inline validateField() into the constructor loop. The method call cost wasn't huge but it's per-field. 2. Skip the rules loop entirely when rules === []. The previous code invoked array_all with a closure even on fields with no rules, which is the common case. 3. Replace array_all(closure) with foreach. Closure allocation per field x per validation = wasted frames. 4. Type-dispatch via match() instead of an if-cascade with seven string comparisons. PHP's match generates a jump table. 5. Resolve values with isset() first, falling through to array_key_exists() only when the key is missing-or-null. Most values aren't null, so the common path is now a single isset. Also replaced reset() (which mutates the array pointer) with array_key_first() in the BC-format detection at the top of the constructor. Numbers (10k iters): Struct construction (8 fields): 24.09us -> 12.55us (1.92x) Struct construction (3 fields): 6.32us -> 5.39us (1.17x) Per-field cost: ~3.0us -> ~1.57us Behavior preserved. Same exceptions thrown for the same conditions (InvalidArgumentException for missing/wrong-type, ValidationException for failed rules). 592/592 tests pass.
Struct construction was spending a chunk of its time on per-instance
schema normalisation work that is identical across every construction
with the same schema: detecting the legacy ['id' => 'int'] format,
defaulting type/nullable/default/rules/alias, computing the "required"
flag. Move it out of the hot loop.
New value object CompiledSchema (final, immutable, public readonly):
\$compiled = CompiledSchema::compile(\$schema);
foreach (\$rows as \$row) {
\$structs[] = new Struct(\$compiled, \$row);
}
Struct::__construct now accepts array|CompiledSchema. Array path is
unchanged from the previous commit — no regression for one-shot
callers. Compiled path goes through a faster initFromCompiled() that
reads pre-resolved field tuples [type, nullable, default, rules,
alias, required] by integer offset.
Numbers (10k iters):
Compile (one-time): ~6.4us
Struct(array) 8 fields: 12.47us
Struct(CompiledSchema) 8 fields: 10.15us (1.23x)
Struct(array) 3 fields: 5.24us
Struct(CompiledSchema) 3 fields: 4.39us (1.19x)
Break-even is roughly 3-5 instances per schema — after that the
compiled path is always cheaper. Cumulative vs the original Struct
implementation (24.09us for 8 fields), the compiled path is 2.37x
faster.
Behavior preserved. Same exceptions, same field-resolution order
(explicit value -> alias -> default), same nullability semantics.
592/592 tests pass.
Attributes\Validator::validateProperty() was calling \$property->getAttributes() and \$attribute->newInstance() on every invocation. For a typical form-request flow that validates the same property across many items in a payload (or many requests in a worker process), the reflection work dominates: ~1.5us per attribute, repeated forever. Cache the parsed attribute instances keyed by 'Class::property'. Property structure doesn't change at runtime, so we instantiate once and reuse. Also added a public static clearCache() for long-running processes that reload classes (rare) and for tests that need a clean slate. Numbers (50k iters): Range (1 attr): 4.60us -> 1.75us (2.62x) Email+NotNull (2 attrs): 12.27us -> 7.85us (1.56x) NotNull+Length (2 attrs): 6.28us -> 3.02us (2.08x) Gain scales with attributes per property — each cached attribute saves ~1.5us of newInstance() reflection work. Also added Tests/Attributes/ValidatorTest.php (5 tests, previously zero coverage on this class). All 597 tests pass.
Same pattern as the earlier Struct optimisation, transplanted to its two siblings. AdvancedStruct (nearly identical to pre-opt Struct): - isset() fast-path for value resolution - skip the rules foreach when rules === [] - match() type dispatch instead of if-cascade - isValidType() made static ImmutableStruct (different shape, narrower wins): - inline nullable detection (\$type[0] === '?') instead of method calls to isNullable() + stripNullable() - only call get_debug_type() when an error is about to be thrown, not eagerly - skip the rules foreach when no rules are declared Numbers (avg of 3x 20k iter runs): AdvancedStruct (8 fields): ~14.5us -> ~8.6us (1.69x) ImmutableStruct (8 fields): ~48.8us -> ~39.0us (1.25x) ImmutableStruct's smaller win reflects irreducible overhead from parent inheritance, separate initialize/setInitial phases, and frozen-state tracking. No structural rewrite — the fixes are mechanical and minimal-risk. Behavior preserved, 597/597 tests pass.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR introduces three optimization features. Validator now caches instantiated attribute objects per Class::property, eliminating reflection overhead on repeated validations. AbstractVector and concrete vector classes (Vec2/Vec3/Vec4) use explicit loops and a trusted-construction flag to avoid re-validating computed arithmetic results. Struct classes integrate a new CompiledSchema type that pre-compiles field definitions for efficient reuse, with refactored validation helpers across Struct, AdvancedStruct, and ImmutableStruct. ChangesValidator Attribute Caching
Vector Arithmetic Optimization
Struct Schema Compilation and Validation Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.55)Composer install failed: the lock file is not up to date with the latest changes in composer.json. Run Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
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 by CodeRabbit
New Features
Bug Fixes
Tests