Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

defineProperty based property memoization #92

Closed
wants to merge 1 commit into from
Closed

Conversation

airhorns
Copy link
Contributor

This implements a better performing getter function for readonly instances. Before this, we used the same definition of the getter function for all views, which ended up megamorphic, because it accessed a very wide variety of properties for every instance! Instead, this evals a monomorphic getter for each one. I also changed the object that stores the memos to have a fixed shape from birth by evaling it out as well.

I also changed us to use one object to store all the memoized values, instead of two. We were previously using two in order to implement memoization of undefined and null correctly, but with the new strategy to initialize an object with slots for every memo from the start, we can populate it with a $notYetMemoized symbol that indicates if we have memoized or not.

Copy link

codspeed-hq bot commented May 31, 2024

CodSpeed Performance Report

Merging #92 will degrade performances by 27.67%

Comparing faster-get-5 (58da4e6) with main (f183291)

Summary

⚡ 4 improvements
❌ 1 regressions
✅ 3 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main faster-get-5 Change
instantiating a large root 4 ms 3.5 ms +14.55%
accessing memoized getter properties of a class model 15.2 µs 10.5 µs +44.43%
accessing memoized null property of a class model 14.4 µs 11.2 µs +28.64%
accessing unmemoized getter properties of a class model 66.6 µs 44.9 µs +48.33%
accessing unmemoized null property of a class model 17.5 µs 24.1 µs -27.67%

@airhorns airhorns changed the title Eval a fast property memoizer to microoptimize property access for cached views defineProperty based property memoization May 31, 2024
…ched views

This implements a better performing getter function for readonly instances. Before this, we used the same definition of the getter function for all views, which ended up megamorphic, because it accessed a very wide variety of properties for every instance! Instead, this evals a monomorphic getter for each one. I also changed the object that stores the memos to have a fixed shape from birth by evaling it out as well.

I also changed us to use one object to store all the memoized values, instead of two. We were previously using two in order to implement memoization of undefined and null correctly, but with the new strategy to initialize an object with slots for every memo from the start, we can populate it with a `$notYetMemoized` symbol that indicates if we have memoized or not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant