Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

N-API: expose function to suggest GC extends external memory #13928

Closed
mhdawson opened this issue Jun 26, 2017 · 3 comments
Closed

N-API: expose function to suggest GC extends external memory #13928

mhdawson opened this issue Jun 26, 2017 · 3 comments
Labels
good first issue Issues that are suitable for first-time contributors. node-api Issues and PRs related to the Node-API.

Comments

@mhdawson
Copy link
Member

mhdawson commented Jun 26, 2017

  • Version: ALL
  • Platform: ALL
  • Subsystem: ALL

Add an api to all the application to give the GC a 'hint' that external memory should be extended. This should be used to optimization and be a no-op on any vm that does not support such a request.

For V8 it would be a wrapper around:

v8::Isolate::AdjustAmountOfExternalAllocatedMemory()

And have a similar implementation to Nan::AdjustExternalMemory().

The N-API team will get to this based on priorities, but this is a good place for others to contribute as well. If you start working on this, assign the issue to yourself and add a comment that you are working on it.

@mscdex mscdex added the node-api Issues and PRs related to the Node-API. label Jun 26, 2017
@addaleax addaleax added the good first issue Issues that are suitable for first-time contributors. label Jun 26, 2017
@chris--young
Copy link

I'd like to work on this

@cjihrig
Copy link
Contributor

cjihrig commented Jul 14, 2017

@chris--young are you still working on this? It looks like you worked on a commit.

@chris--young
Copy link

@cjihrig yea sorry for the delay, been pretty busy the past couple weeks. I just opened a pr for this #14310

chris--young pushed a commit to chris--young/node that referenced this issue Aug 25, 2017
Added a simple wrapper around v8::Isolate::AdjustAmountOfExternalAllocatedMemory

Fixes: nodejs#13928
chris--young pushed a commit to chris--young/node that referenced this issue Aug 25, 2017
Improved documentation based on pull request feedback

Fixes: nodejs#13928
chris--young pushed a commit to chris--young/node that referenced this issue Aug 25, 2017
Changed napi_adjust_external_memory() signature to be more
consistent with the rest of the API

Fixes: nodejs#13928
chris--young pushed a commit to chris--young/node that referenced this issue Aug 25, 2017
cjihrig pushed a commit to cjihrig/node that referenced this issue Aug 31, 2017
Added a wrapper around
v8::Isolate::AdjustAmountOfExternalAllocatedMemory

PR-URL: nodejs#14310
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Fixes: nodejs#13928
addaleax pushed a commit to addaleax/ayo that referenced this issue Sep 5, 2017
Added a wrapper around
v8::Isolate::AdjustAmountOfExternalAllocatedMemory

PR-URL: nodejs/node#14310
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Fixes: nodejs/node#13928
MylesBorins pushed a commit that referenced this issue Sep 10, 2017
Added a wrapper around
v8::Isolate::AdjustAmountOfExternalAllocatedMemory

PR-URL: #14310
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Fixes: #13928
MylesBorins pushed a commit that referenced this issue Sep 12, 2017
Added a wrapper around
v8::Isolate::AdjustAmountOfExternalAllocatedMemory

PR-URL: #14310
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Fixes: #13928
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Apr 10, 2018
Added a wrapper around
v8::Isolate::AdjustAmountOfExternalAllocatedMemory

PR-URL: nodejs#14310
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Fixes: nodejs#13928
MylesBorins pushed a commit that referenced this issue Apr 16, 2018
Added a wrapper around
v8::Isolate::AdjustAmountOfExternalAllocatedMemory

Backport-PR-URL: #19447
PR-URL: #14310
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Fixes: #13928
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are suitable for first-time contributors. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants