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 constructor #66

Merged
merged 1 commit into from
Nov 9, 2023
Merged

Conversation

airhorns
Copy link
Contributor

@airhorns airhorns commented Nov 7, 2023

On top of #65

15% improvement by removing a megamorphic call to the fast instantiator function! Yeesh!

On perf-improvements:

❯ node dist/bench/create-many-different-roots.js
instantiating a large root x 3,258 ops/sec ±0.84% (100 runs sampled)
instantiating a small root x 10,573,575 ops/sec ±0.21% (92 runs sampled)
instantiating a diverse root x 464,002 ops/sec ±0.15% (100 runs sampled)

On this branch:

❯ node dist/bench/create-many-different-roots.js
instantiating a large root x 3,851 ops/sec ±0.29% (97 runs sampled)
instantiating a small root x 15,595,988 ops/sec ±0.46% (99 runs sampled)
instantiating a diverse root x 582,571 ops/sec ±0.65% (99 runs sampled)

@airhorns airhorns force-pushed the fast-constructor-cheap-super branch from ed8b219 to a75036e Compare November 8, 2023 00:06
@airhorns airhorns changed the title fast constructor cheap super Faster constructor Nov 8, 2023
@airhorns airhorns changed the base branch from main to perf-improvements November 8, 2023 00:08
this[$env] = context.env;
this[$parent] = parent;

(this.constructor as IClassModelType<any>)[$fastInstantiator](this as any, snapshot, context);
Copy link
Contributor Author

@airhorns airhorns Nov 8, 2023

Choose a reason for hiding this comment

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

This call was megamorphic before -- all the MQT classes subclass this one, and then this class ends up calling the instantiator. Because they all share the same parent class, this callsite is shared among all of em! Instead of generating this function, we can instead generate a child class that hardcodes the instantiation logic right in its constructor. Super duper monomorphic!

Copy link
Contributor

Choose a reason for hiding this comment

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

So simple, but so effective!

Base automatically changed from perf-improvements to main November 8, 2023 15:32
@airhorns airhorns marked this pull request as ready for review November 9, 2023 14:58
@airhorns airhorns requested a review from thegedge November 9, 2023 14:58
@airhorns airhorns changed the base branch from main to upgrade-node November 9, 2023 14:58
@airhorns airhorns force-pushed the fast-constructor-cheap-super branch from a75036e to 1490d2b Compare November 9, 2023 14:59
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.

Majestic!

this[$env] = context.env;
this[$parent] = parent;

(this.constructor as IClassModelType<any>)[$fastInstantiator](this as any, snapshot, context);
Copy link
Contributor

Choose a reason for hiding this comment

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

So simple, but so effective!

@airhorns airhorns merged commit 1678862 into upgrade-node Nov 9, 2023
4 checks passed
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