lib: remove unnecessary bind#28131
Closed
lib: remove unnecessary bind #28131
Uh oh!
There was an error while loading. Please reload this page.
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.
apapirovskiforce-pushed the remove-unnecessary-bind branch from 9c884b8 to e51c91cCompareCollaborator
nodejs-github-bot commented Jun 8, 2019
lpinca reviewed Jun 8, 2019
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Collaborator
nodejs-github-bot commented Jun 8, 2019
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
addaleax commented Aug 25, 2019
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.
apapirovskiforce-pushed the remove-unnecessary-bind branch from e51c91c to c7a87faCompare
ContributorAuthor
Well, this is a blast from the past. Thought I merged it... |
apapirovskiforce-pushed the remove-unnecessary-bind branch from c7a87fa to 193f0c2CompareCollaborator
nodejs-github-bot commented Dec 13, 2019
apapirovskiforce-pushed the remove-unnecessary-bind branch from 193f0c2 to d077f00CompareCollaborator
nodejs-github-bot commented Dec 13, 2019
BridgeAR approved these changes Dec 14, 2019
Collaborator
nodejs-github-bot commented Dec 14, 2019
Member
Trott commented Dec 14, 2019
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
PR-URL: #28131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
PR-URL: #28131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
targos pushed a commit that referenced this pull request Jan 14, 2020
PR-URL: #28131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
targos pushed a commit that referenced this pull request Jan 14, 2020
PR-URL: #28131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
PR-URL: #28131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
PR-URL: #28131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Merged
Sign up for freeto 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