smalloc: do not track external memory #1375
Closed
Conversation
This file contains 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
The memory that was allocated outside of the `smalloc.cc` should not be tracked using `AdjustAmountOfExternalAllocatedMemory`. There are no potential issues except triggering V8's GC way too often. In fact, `heap.js` is creating a buffer out of the pointer, and since it doesn't know the size of the pointer - it just creates the maximum possible `Buffer` instance with a no-op free callback and no hint.
indutny
force-pushed
the
fix/smalloc-ownership-track
branch
from
882c657 to
1407d99
Compare
| callback_(static_cast<char*>(array_data), hint_); | ||
| isolate->AdjustAmountOfExternalAllocatedMemory( | ||
| -static_cast<int64_t>(sizeof(*this))); |
There was a problem hiding this comment.
It's arguably better to call Isolate::AdjustAmountOfExternalAllocatedMemory() only once. As is, the code may potentially trigger a GC twice.
|
LGTM if the CI is happy. |
|
Thank you! |
|
win2008r2 is just timers defying.. time. Again. |
|
I guess we are good to go? |
indutny
added a commit
that referenced
this pull request
Apr 8, 2015
The memory that was allocated outside of the `smalloc.cc` should not be tracked using `AdjustAmountOfExternalAllocatedMemory`. There are no potential issues except triggering V8's GC way too often. In fact, `heap.js` is creating a buffer out of the pointer, and since it doesn't know the size of the pointer - it just creates the maximum possible `Buffer` instance with a no-op free callback and no hint. PR-URL: #1375 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
Landed in ff74931, thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The memory that was allocated outside of the
smalloc.ccshould not betracked using
AdjustAmountOfExternalAllocatedMemory. There are nopotential issues except triggering V8's GC way too often.
In fact,
heap.jsis creating a buffer out of the pointer, and since itdoesn't know the size of the pointer - it just creates the maximum
possible
Bufferinstance with a no-op free callback and no hint.R=@trevnorris or @bnoordhuis