Reuse one AttributesIterator per validator; drop Enumerable#2726
Open
ericproulx wants to merge 1 commit into
Open
Reuse one AttributesIterator per validator; drop Enumerable#2726ericproulx wants to merge 1 commit into
ericproulx wants to merge 1 commit into
Conversation
AttributesIterator's attrs/scope are static per validator; only params varies per request. Build the iterator once in Base#initialize (frozen) via an overridable iterator_class, and thread params through #each instead of storing request-derived state in @original_params/@params. The reused instance is shared across threads, so it must hold no mutable per-request state. - Remove `include Enumerable` (dead + misleading: #each never returned an Enumerator and nothing used map/select/etc.) and the unused `attr_reader :scope`. - MultipleParamsBase#validate! now lazily builds array_errors (`nil` -> `||= []`) like Base, avoiding an empty-array allocation on the no-error path. Allocation: one fewer object (~80 B) per validator per request; speed-neutral. Full validation/endpoint suites green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
b7da330 to
a55438b
Compare
Danger ReportNo issues found. |
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
AttributesIterator'sattrs/scopeare static per validator; onlyparamsvaries per request. Yet a fresh iterator was allocated per validator per request invalidate!, with the request-derived state (@original_params/@params) computed in the constructor.This builds the iterator once in
Base#initialize(frozen) and threadsparamsthrough#each, so the instance carries no per-request state and can be reused. It also drops the unusedEnumerablemixin.What changed
AttributesIteratorinclude Enumerable— dead and misleading:#eachnever returned anEnumerator(noenum_forfallback) and nothing in the codebase usedmap/select/to_a/etc. Only#eachis ever called.attr_reader :scope— zero callers in lib/ or spec/.(attrs, scope).paramsflows througheach(params)→do_each(..., original_params, ...)as an argument instead of the@original_params/@paramsivars.Base— overridable privateiterator_class(defaultSingleAttributeIterator);@iterator = iterator_class.new(@attrs, @scope).freezebuilt in#initialize;#validate!uses@iterator.each(params).MultipleParamsBase— overridesiterator_class→MultipleAttributesIterator;#validate!now buildsarray_errorslazily (nil→||= []) likeBase, avoiding an empty-array allocation on the no-error path.DefaultValidator— uses the shared@iterator.new(attrs, scope)+each(params, &b).The reused instance holds only the static, frozen
@attrs/@scopeand writes no ivars during traversal — safe under Grape's frozen, thread-shared validators.Benchmark
Real pre-change implementation (from
master, identicaldo_eachmachinery) vs. the new one, one validator traversal:Speed is neutral (ips delta within noise —
scope.params/Array.wrapstill run per request, just ineachinstead of the ctor). The reliable win is one fewer object (~80 B) per validator per request — a GC-pressure reduction that scales with#validators × throughput, in the spirit of #2689/#2690. This is an allocation/clarity refactor, not a throughput optimization.Testing
spec/grape/validations+spec/grape/endpoint_spec.rb+spec/grape/api_spec.rb: 0 failures🤖 Generated with Claude Code