streams: Ensure that instanceof fast-path is hit. by bmeurer · Pull Request #13403 · nodejs/node
With the new Ignition+TurboFan pipeline, the instanceof fast-path can be missed if the right-hand side needs a TDZ check, i.e. is const declared on a surrounding scope. This doesn't apply to Node 8 at this point, where it's at V8 5.8, but it applies as soon as V8 5.9 rolls. There's work going on in Ignition (and TurboFan) to optimize those TDZ checks properly, but those changes will land in V8 6.1, so might not end up in Node 8. One way to work-around this in Node core for now is to use var instead of const for those right-hand sides for instanceof for now, especially Buffer in case of streams. This is not beautiful, but proper ducktape. Improves readable-bigread.js by ~23% with Node LKGR.
addaleax pushed a commit that referenced this pull request
Jun 9, 2017With the new Ignition+TurboFan pipeline, the instanceof fast-path can be missed if the right-hand side needs a TDZ check, i.e. is const declared on a surrounding scope. This doesn't apply to Node 8 at this point, where it's at V8 5.8, but it applies as soon as V8 5.9 rolls. There's work going on in Ignition (and TurboFan) to optimize those TDZ checks properly, but those changes will land in V8 6.1, so might not end up in Node 8. One way to work-around this in Node core for now is to use var instead of const for those right-hand sides for instanceof for now, especially Buffer in case of streams. This is not beautiful, but proper ducktape. Improves readable-bigread.js by ~23% with Node LKGR. PR-URL: #13403 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Calvin Metcalf <calvin.metcalf@gmail.com>
addaleax pushed a commit that referenced this pull request
Jun 10, 2017With the new Ignition+TurboFan pipeline, the instanceof fast-path can be missed if the right-hand side needs a TDZ check, i.e. is const declared on a surrounding scope. This doesn't apply to Node 8 at this point, where it's at V8 5.8, but it applies as soon as V8 5.9 rolls. There's work going on in Ignition (and TurboFan) to optimize those TDZ checks properly, but those changes will land in V8 6.1, so might not end up in Node 8. One way to work-around this in Node core for now is to use var instead of const for those right-hand sides for instanceof for now, especially Buffer in case of streams. This is not beautiful, but proper ducktape. Improves readable-bigread.js by ~23% with Node LKGR. PR-URL: #13403 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Calvin Metcalf <calvin.metcalf@gmail.com>
kisg pushed a commit to paul99/v8mips that referenced this pull request
Jun 13, 2017Port the baseline implementation of Object.prototype.isPrototypeOf to the CodeStubAssembler, sharing the existing prototype chain lookup logic with the instanceof / OrdinaryHasInstance implementation. Based on that, do the same in TurboFan, introducing a new JSHasInPrototypeChain operator, which encapsulates the central prototype chain walk logic. This speeds up Object.prototype.isPrototypeOf by more than a factor of four, so that the code A.prototype.isPrototypeOf(a) is now performance-wise on par with a instanceof A for the case where A is a regular constructor function and a is an instance of A. Since instanceof does more than just the fundamental prototype chain lookup, it was discovered in Node core that O.p.isPrototypeOf would be a more appropriate alternative for certain sanity checks, since it's less vulnerable to monkey-patching. In addition, the Object builtin would also avoid the performance-cliff associated with instanceof (due to the Symbol.hasInstance hook), as for example hit by nodejs/node#13403 (comment). The main blocker was the missing performance of isPrototypeOf, since it was still a JS builtin backed by a runtime call. This CL also adds more test coverage for the Object.prototype.isPrototypeOf builtin, especially when called from optimized code. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng BUG=v8:5269,v8:5989,v8:6483 R=jgruber@chromium.org Review-Url: https://codereview.chromium.org/2934893002 Cr-Commit-Position: refs/heads/master@{#45925}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters