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

Faster cached view version 7: values right on the instance #94

Merged
merged 1 commit into from
May 31, 2024

Conversation

airhorns
Copy link
Contributor

@airhorns airhorns commented May 31, 2024

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 the object itself to store all the memoized values, instead of two other tracker objects. We were previously using the trackers in order to implement memoization of undefined and null correctly. Now, we set up symbol properties to store all the memos, but on the prototype of the class model. This means we don't spend any time setting them up during construction, but the JS VM knows the shape of the memos ahead of time! Less memory and same speed woop woop! Then, when memoizing, we can stash the value under that symbol key, which means fewer property accesses and lookups to fetch it.

Replaces #90

@airhorns airhorns changed the title Eval a fast property memoizer to microoptimize property access for cached views Faster cached view version 7: values right on the instance May 31, 2024
@airhorns airhorns requested a review from thegedge May 31, 2024 16:27
Copy link

codspeed-hq bot commented May 31, 2024

CodSpeed Performance Report

Merging #94 will improve performances by 14.04%

Comparing faster-get-7 (998d49a) with main (f183291)

Summary

⚡ 2 improvements
✅ 6 untouched benchmarks

Benchmarks breakdown

Benchmark main faster-get-7 Change
instantiating a large union 91.3 µs 80.1 µs +14.04%
accessing unmemoized null property of a class model 17.4 µs 15.4 µs +12.86%

…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 the object itself to store all the memoized values, instead of two other tracker objects. We were previously using the trackers in order to implement memoization of undefined and null correctly. Now, we set up symbol properties to store all the memos, but on the prototype of the class model. This means we don't spend any time setting them up during construction, but the JS VM knows the shape of the memos ahead of time! Less memory and same speed woop woop! Then, when memoizing, we can stash the value under that symbol key, which means fewer property accesses and lookups to fetch it.
@airhorns airhorns removed the request for review from thegedge May 31, 2024 16:50
@airhorns airhorns marked this pull request as draft May 31, 2024 16:50
@airhorns airhorns marked this pull request as ready for review May 31, 2024 17:22
@airhorns airhorns requested a review from thegedge May 31, 2024 17:22
Copy link
Contributor

@thegedge thegedge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 👏

}

value = getValue.call(this);
this[$memo] = value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌎 🚀 🌔

@airhorns airhorns merged commit a32d11b into main May 31, 2024
6 checks passed
@airhorns airhorns deleted the faster-get-7 branch May 31, 2024 18:09
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.

2 participants