lib: remove unnecessary bind #28131
Closed
lib: remove unnecessary bind #28131
Conversation
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
This comment has been minimized.
This comment has been minimized.
nodejs-github-bot added the lib / src This comment has been minimized.
This comment has been minimized.
apapirovski force-pushed the remove-unnecessary-bind branch from 9c884b8 to e51c91c Compare Collaborator
lpinca reviewed Jun 8, 2019
Collaborator
lpinca approved these changes Jun 8, 2019
jasnell approved these changes Jun 10, 2019
cjihrig approved these changes Jun 10, 2019
Trott approved these changes Jun 10, 2019
Member
| ping @apapirovski :) |
tniessen approved these changes Oct 30, 2019
process.nextTick accepts additional parameters which are passed through to the callback. Use that instead of binding the function to a context.
Pass through parameters using setImmediate rather than using Function.prototype.bind to bind the provided context.
Don't use Function.prototype.bind where it isn't necessary. Rely on event emitter context instead and on arrow function as class property.
apapirovski force-pushed the remove-unnecessary-bind branch from e51c91c to c7a87fa Compare 
Contributor Author
| Well, this is a blast from the past. Thought I merged it... |
apapirovski force-pushed the remove-unnecessary-bind branch from c7a87fa to 193f0c2 Compare Collaborator
apapirovski force-pushed the remove-unnecessary-bind branch from 193f0c2 to d077f00 Compare Collaborator
BridgeAR approved these changes Dec 14, 2019
Collaborator
Member
| Landed in e17403e...2e738fb |
Trott pushed a commit that referenced this pull request Dec 14, 2019
process.nextTick accepts additional parameters which are passed through to the callback. Use that instead of binding the function to a context. PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Trott pushed a commit that referenced this pull request Dec 14, 2019
Pass through parameters using setImmediate rather than using Function.prototype.bind to bind the provided context. PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
Pass through parameters using setImmediate rather than using Function.prototype.bind to bind the provided context. PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
Don't use Function.prototype.bind where it isn't necessary. Rely on event emitter context instead and on arrow function as class property. PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Merged
targos pushed a commit that referenced this pull request Jan 14, 2020
process.nextTick accepts additional parameters which are passed through to the callback. Use that instead of binding the function to a context. PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Jan 14, 2020
Pass through parameters using setImmediate rather than using Function.prototype.bind to bind the provided context. PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Jan 14, 2020
Don't use Function.prototype.bind where it isn't necessary. Rely on event emitter context instead and on arrow function as class property. PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Jan 14, 2020
PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Jan 14, 2020
PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
process.nextTick accepts additional parameters which are passed through to the callback. Use that instead of binding the function to a context. PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
Pass through parameters using setImmediate rather than using Function.prototype.bind to bind the provided context. PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
Don't use Function.prototype.bind where it isn't necessary. Rely on event emitter context instead and on arrow function as class property. PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
PR-URL: #28131 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
There has been a lot of effort on the V8 side to optimize
bindperformance but nonetheless it's still unnecessary overhead in many cases. Removing it can also make the resulting code more readable as the implied scope isn't immediately apparent when reading certain functions.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes