src: cache and resue isolate and contex pointers#23024
Closed
gireeshpunathil wants to merge 1 commit intonodejs:masterfrom
Closed
src: cache and resue isolate and contex pointers#23024gireeshpunathil wants to merge 1 commit intonodejs:masterfrom
gireeshpunathil wants to merge 1 commit intonodejs:masterfrom
Conversation
Many places these values are obtained through `env` pointer that incurs function calls. Source in once and re-use as much as possible. There are more of this pattern in this file, but those are either one-time use or used in conditional blocks which means sourcing-in those data early may not be beneficial.
nodejs-github-bot
added
c++
addaleax
approved these changes
Sep 22, 2018
Member
There was a problem hiding this comment.
@gireeshpunathil I’m not sure … the commit message makes it sound like this is about performance? These calls should all be essentially free in terms of runtime cost, so this is mostly a question of readability, and I’m not sure it’s worth the churn if the goal is to apply these kinds of changes everywhere…
In any case, this PR LGTM if you’re volunteering to backport this as soon as it lands :)
Member
Author
|
sure, thanks @addaleax . runtime cost was a consideration which I thought of it first, but your point makes sense to me now as these calls boils down to mere field accesses. |
jasnell
approved these changes
Sep 22, 2018
devsnek
approved these changes
Sep 22, 2018
Member
addaleax
added
the
author ready
Contributor
|
Landed in ea3bb9a. |
danbev
pushed a commit
that referenced
this pull request
Sep 25, 2018
Many places these values are obtained through `env` pointer that incurs function calls. Source in once and re-use as much as possible. There are more of this pattern in this file, but those are either one-time use or used in conditional blocks which means sourcing-in those data early may not be beneficial. PR-URL: #23024 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host>
targos
pushed a commit
that referenced
this pull request
Sep 25, 2018
Many places these values are obtained through `env` pointer that incurs function calls. Source in once and re-use as much as possible. There are more of this pattern in this file, but those are either one-time use or used in conditional blocks which means sourcing-in those data early may not be beneficial. PR-URL: #23024 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host>
This was referenced Oct 10, 2018
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 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.
Many places these values are obtained through
envpointer that incurs function calls. Source in once and re-use as much as possible. There are more of this pattern in this file, but those are eitherone-time use or used in conditional blocks which means sourcing-in those data early may not be beneficial.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes