perf: add fast path to OrdinaryGet#5413
perf: add fast path to OrdinaryGet#5413Serdnad wants to merge 3 commits intoboa-dev:mainboa-dev/boa:mainfrom Serdnad:perf/ordinary-get-data-fast-pathSerdnad/boa:perf/ordinary-get-data-fast-pathCopy head branch name to clipboard
Conversation
Avoid constructing a PropertyDescriptor for the common case of reading a plain data property. `ordinary_get` now resolves named own-property lookups on ordinary objects in a single shape probe via `PropertyMap::get_own_data_named`, reading the value directly from storage and going straight to the prototype walk on a miss (no redundant second lookup). Gated on the object using the ordinary `__get_own_property__`, so accessors, indexed keys and exotic objects fall back to the spec path. Benchmarks vs baseline: regexp -5.9%, earley-boyer -3.7%, raytrace -3.2%; properties/access -10%, prototypes/chain -10%, strings -12..-23%.
Test262 conformance changes
Tested main commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5413 +/- ##
===========================================
+ Coverage 47.24% 62.72% +15.48%
===========================================
Files 476 535 +59
Lines 46892 59112 +12220
===========================================
+ Hits 22154 37078 +14924
+ Misses 24738 22034 -2704 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| // Accessor: fall through to invoke the getter. | ||
| OwnDataLookup::Accessor => {} | ||
| } | ||
| } |
There was a problem hiding this comment.
Wait, why does this improve performance, if it also needs to construct an OwnDataLookup struct? I'm not criticizing the approach, I want to understand this because we might have the same perf benefits of this with #5284, which doesn't need to specialize property lookups.
There was a problem hiding this comment.
Hey @jedel1043 , thanks for taking a look at my PR, and apologies for the slow response.
I took a look at #5284 , and yes, the premise is effectively the same; the proposed enum OwnDataLookup is slightly smaller and cheaper to use than PropertyDescriptor, and since this is a hot path, it's enough to have an impact.
That said, I also took the liberty of benchmarking the other PR beside mine, and while they're very similar (similar improvements/regressions on the same benchmarks), which is to be expected, there are one or two v8 bench outliers where the other PR significantly beats mine. With that in mind, would be fully in support of closing my PR in favor of the other if you prefer - I hadn't seen it before opening mine.
There was a problem hiding this comment.
Did you also test your changes on top of #5284? Because it might be cool if they composed together to give a bigger perf increase. If the difference is essentially nil, we can close this.
Adds a fast path to
OrdinaryGetthat reads a plain data property's value directly from object storage, skippingPropertyDescriptorconstruction.Benchmarks on local machine: