Skip to content

Commit

Permalink
perf(Subscription): use instanceof to avoid megamorphic LoadIC (#4499)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bmeurer authored and benlesh committed Jan 29, 2019
1 parent fb24303 commit 065b4e3
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/internal/Subscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export class Subscription implements SubscriptionLike {
} else if (this.closed) {
subscription.unsubscribe();
return subscription;
} else if (typeof subscription._addParent !== 'function' /* quack quack */) {
} else if (!(subscription instanceof Subscription)) {
const tmp = subscription;
subscription = new Subscription();
subscription._subscriptions = [tmp];
Expand Down

0 comments on commit 065b4e3

Please sign in to comment.