Speed up the Parameter get/set hot paths - #1167
Open
philippjfr wants to merge 5 commits into
Open
Conversation
The existing suite covered class creation, .param access, depends and watcher triggering, but not the plain value get/set descriptor paths, which are the hottest operations in param. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Parameter.__getattribute__ is a Python-level override, so every slot read on a Parameter is materially more expensive than a plain attribute read (it also defeats CPython's adaptive LOAD_ATTR specialization). __get__ read 'name' twice and __set__ read it five times. Bind 'name' and 'readonly' to locals, hoist the repeated _param__private descriptor lookups, and only read the 'default' slot when there is no stored value to fall back from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Number.__get__ called super().__get__() to resolve the value and then _value_is_dynamic(), which walked the whole descriptor chain a second time just to test the stored value for a generator. This addresses the existing TODO on that method. Add Dynamic._resolve_dynamic(), which returns (value, is_dynamic) from a single fetch, and use it from both Dynamic.__get__ and Number.__get__. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1167 +/- ##
==========================================
+ Coverage 86.73% 86.75% +0.02%
==========================================
Files 9 9
Lines 5321 5330 +9
==========================================
+ Hits 4615 4624 +9
Misses 706 706 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
Author
|
Full asv benchmark output comparing before (the commit adding more benchmarks) and the final commit: |
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
Parameter.__get__andParameter.__set__are the hottest code in param: everyattribute read and write on a
Parameterizedinstance goes through them, anddownstream libraries like Panel do that millions of times in a session. This PR
adds asv coverage for those paths (there was none), then removes two sources of
redundant work that the coverage exposed.
Reads of a
Numberparameter are now roughly 2x faster. Reads of otherparameter types improve by 17-38%, and writes by 3-14%. No behaviour changes, no
API changes, no new slots, and the full test suite passes unchanged
(1514 passed, 4 skipped, 2 xfailed).
Changes
bench: Add asv benchmarks for parameter value get/set hot pathsThe existing suite covered class creation,
.paramaccess,dependsand watchertriggering, but had no benchmark for plain parameter value get/set. Adds four
suites:
ParameterizedGetValueSuiteandParameterizedSetValueSuite(parameterized over
Parameter,Number,String,Boolean, at both class andinstance level),
ParameterizedFirstSetValueSuite(the one-off cost of the firstset, which triggers the per-instance Parameter copy), and
ParameterUnboundSlotSuite(slot reads on an unbound Parameter, with and without_slot_defaultsfallback).The get/set suites warm up in
setupso they measure the steady state ratherthan first-touch instantiation.
perf: Avoid redundant Parameter slot reads in __get__ and __set__Slot reads on a Parameter are not free.
Parameter.__getattribute__isoverridden (
parameterized.py:1888) to resolveUndefinedslots against_slot_defaults, which costs about 55ns per read instead of the ~13ns anunoverridden attribute read would cost, because a Python-level
__getattribute__defeats CPython's adaptiveLOAD_ATTRspecialization.__get__readself.nametwice and__set__read it five times. Both now bindit to a local once.
__set__also readself.defaulteagerly even on the commonpath where a stored value already exists, and re-read
obj._param__privaterepeatedly; the
defaultread now happens only in theKeyErrorbranch wherethere is no stored value, and
_param__privateis bound to a local.perf: Resolve Number value and dynamism in a single passNumber.__get__calledsuper().__get__()to resolve the value, then_value_is_dynamic()to decide whether to re-validate, and that second callwalked the entire descriptor chain a second time. There was already a maintainer
TODO on the method saying as much.
Adds
Dynamic._resolve_dynamic(), which returns(value, is_dynamic)from asingle traversal.
Dynamic.__get__is now a thin wrapper over it, andNumber.__get__uses it directly. The dynamism probe also checkscallable(gen)before
hasattr(gen, '_Dynamic_last'), since generators are always callable andthe cheap check short-circuits the common static-value case.
_value_is_dynamicis untouched and still used by its other callers.Benchmarks
asvon macOS / arm64, each commit built and run in its own isolatedenvironment. Baseline is the commit that adds the benchmarks.
Reads:
GetValue.time_instance('Number')GetValue.time_class('Number')GetValue.time_instance('Boolean')GetValue.time_instance('Parameter')GetValue.time_instance('String')UnboundSlot.time_undefined_slotWrites:
SetValue.time_instance('Parameter')SetValue.time_instance('String')SetValue.time_instance('Boolean')SetValue.time_instance('Number')SetValue.time_class('Number')WatcherSuite.time_triggerAttribution: the slot-read commit is the broad but shallow win (every read
0.77-0.83,
Numberreads 688 to 563ns,set('Parameter')1.52 to 1.31μs). The_resolve_dynamiccommit is where theNumberhalving comes from (563 to 334nsinstance, 688 to 417ns class).
No regressions.
ParameterizedParamAccessSuite.time_classreports 125 to 166nsin the second step, but that operation measures 28.6ns in-process, well below
asv's ~42ns timer granularity on this machine, and it was flat across the first
step. That granularity is also why so many sub-microsecond figures land on
exactly 83/125/167/208/250ns; the
Numberread result is the one sub-microsecondnumber comfortably outside it, and it is corroborated in-process at 671 to 206ns.
AI Disclosure
PR created with heavy help from Claude Opus 5.