Skip to content

Commit 065b4e3

Browse files
bmeurerbenlesh
authored andcommitted
perf(Subscription): use instanceof to avoid megamorphic LoadIC (#4499)
In `Subscription#add()` use `instanceof` to test whether `subscription` must be wrapped in a `Subscription` (aka for presence of `_addParent` method), instead of `typeof subscription._addParent === 'function'`, as the latter is going to turn into a *megamorphic property access* in any realistic application and cause quite a bit of contention on the megamorphic stub cache in Node (aka V8). The `instanceof` is definitely faster, even if the property access would hit the megamorphic stub cache. For example in the case of a simple [Angular Universal prerender][1] test, just changing this can reduce the number of megamorphic stub cache misses by around **2%**. Even in the case where the megamorphic stub cache hits all the time, using `instanceof` is still faster. I put together a little micro- benchmark [here][2]. Running this with the latest V8 we get: ``` $ out/Release/d8 bench-instanceof-versus-typeof-megamorphic.js instanceOf: 91 ms. typeOf: 138 ms. ``` [1]: https://gist.githubusercontent.com/bmeurer/5b9480ef1a74c5187180193abc73dcd4/raw/fc5d9a01a6b7fe841efa1c7773ae55e70435d16a/prerender.js [2]: https://gist.github.com/bmeurer/d6d8d4e5d3a9937499ed738c93b3b7cf
1 parent fb24303 commit 065b4e3

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

src/internal/Subscription.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ export class Subscription implements SubscriptionLike {
154154
} else if (this.closed) {
155155
subscription.unsubscribe();
156156
return subscription;
157-
} else if (typeof subscription._addParent !== 'function' /* quack quack */) {
157+
} else if (!(subscription instanceof Subscription)) {
158158
const tmp = subscription;
159159
subscription = new Subscription();
160160
subscription._subscriptions = [tmp];

0 commit comments

Comments
 (0)