-
Notifications
You must be signed in to change notification settings - Fork 28.9k
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
src,lib: add constrainedMemory API for process #46218
src,lib: add constrainedMemory API for process #46218
Conversation
Review requested:
|
7b17275
to
5540f21
Compare
5540f21
to
3fd6390
Compare
Could also add some docs? |
Oh, sorry I forgot, fixed. |
fd857a6
to
262ae5e
Compare
doc/api/process.md
Outdated
is unknown, `0` is returned. | ||
|
||
It is not unusual for this value to be less than or greater than `os.totalmem()`. | ||
This function currently only returns a non-zero value on Linux, based on cgroups |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence is worded a bit confusingly. What is the value on Windows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description was copied from the Libuv documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That may be true but it's still confusing as is. Not sure I have a better suggestion tho
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two suggestions:
-
Maybe add an
Experimental
marker to the docs. Libuv's implementation changed in the not too distant past and it may change again -
Implementing it as a fast API is probably overkill. The limiting factor is the amount of file parsing libuv has to do. And also, you're probably not going to call it 1,000x/sec.
Thanks for suggestions, Hi @anonrig, would you mind taking a look at the second suggestion. Thanks ! |
I agree that it is an overkill, but I don't see any downside of adding fast API. This might be important/valuable functionality which result in high usage (for example APMs). But if @bnoordhuis or anybody disagrees with it, I'm fine with removing it. |
262ae5e
to
c5d496f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any downside of adding fast API
From what I read in #46199, the fast API would only be triggered if the function is called in a loop with a high iteration count. Unless that's something we expect, the fast API would be a function with little to no usage.
1ac7401
to
fca8b25
Compare
Thanks for all suggestions, i have removed the fast API code, please help review again. |
fca8b25
to
798d817
Compare
@tniessen Hi, can help this PR again ? thanks . |
There weren't any objections, blockers and/or active conversations. We can merge it. |
Landed in 73c0564 |
PR-URL: #46218 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
Notable changes: buffer: * (SEMVER-MINOR) add isAscii method (Yagiz Nizipli) #46046 deps: * upgrade npm to 9.4.0 (npm team) #46353 esm: * leverage loaders when resolving subsequent loaders (Maël Nison) #43772 fs: * (SEMVER-MINOR) add statfs() functions (Colin Ihrig) #46358 src,lib: * (SEMVER-MINOR) add constrainedMemory API for process (theanarkh) #46218 test_runner: * (SEMVER-MINOR) add reporters (Moshe Atlow) #45712 v8: * (SEMVER-MINOR) support gc profile (theanarkh) #46255 vm: * (SEMVER-MINOR) expose cachedDataRejected for vm.compileFunction (Anna Henningsen) #46320 PR-URL: #46455
Notable changes: buffer: * (SEMVER-MINOR) add isAscii method (Yagiz Nizipli) #46046 deps: * upgrade npm to 9.4.0 (npm team) #46353 esm: * leverage loaders when resolving subsequent loaders (Maël Nison) #43772 fs: * (SEMVER-MINOR) add statfs() functions (Colin Ihrig) #46358 src,lib: * (SEMVER-MINOR) add constrainedMemory API for process (theanarkh) #46218 test_runner: * (SEMVER-MINOR) add reporters (Moshe Atlow) #45712 v8: * (SEMVER-MINOR) support gc profile (theanarkh) #46255 vm: * (SEMVER-MINOR) expose cachedDataRejected for vm.compileFunction (Anna Henningsen) #46320 PR-URL: #46455
PR-URL: #46218 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
Notable Changes buffer: * add isAscii method (Yagiz Nizipli) #46046 fs: * add statfs() functions (Colin Ihrig) #46358 src,lib: * add constrainedMemory API for process (theanarkh) #46218 v8: * support gc profile (theanarkh) #46255 vm: * expose cachedDataRejected for vm.compileFunction (Anna Henningsen) #46320 PR-URL: TDB
Notable Changes buffer: * add isAscii method (Yagiz Nizipli) #46046 fs: * add statfs() functions (Colin Ihrig) #46358 src,lib: * add constrainedMemory API for process (theanarkh) #46218 v8: * support gc profile (theanarkh) #46255 vm: * expose cachedDataRejected for vm.compileFunction (Anna Henningsen) #46320 PR-URL: #46920
Notable Changes: buffer: * add isAscii method (Yagiz Nizipli) #46046 fs: * add statfs() functions (Colin Ihrig) #46358 src,lib: * add constrainedMemory API for process (theanarkh) #46218 v8: * support gc profile (theanarkh) #46255 vm: * expose cachedDataRejected for vm.compileFunction (Anna Henningsen) #46320 PR-URL: #46920
Notable Changes: buffer: * (SEMVER-MINOR) add isAscii method (Yagiz Nizipli) #46046 doc,lib,src,test: * rename --test-coverage (Colin Ihrig) #46017 fs: * (SEMVER-MINOR) add statfs() functions (Colin Ihrig) #46358 src,lib: * (SEMVER-MINOR) add constrainedMemory API for process (theanarkh) #46218 test_runner: * add initial code coverage support (Colin Ihrig) #46017 * (SEMVER-MINOR) add reporters (Moshe Atlow) #45712 v8: * (SEMVER-MINOR) support gc profile (theanarkh) #46255 vm: * (SEMVER-MINOR) expose cachedDataRejected for vm.compileFunction (Anna Henningsen) #46320 PR-URL: #46920
PR-URL: #46218 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
Notable changes: buffer: * (SEMVER-MINOR) add isAscii method (Yagiz Nizipli) #46046 doc,lib,src,test: * rename --test-coverage (Colin Ihrig) #46017 fs: * (SEMVER-MINOR) add statfs() functions (Colin Ihrig) #46358 src,lib: * (SEMVER-MINOR) add constrainedMemory API for process (theanarkh) #46218 test_runner: * add initial code coverage support (Colin Ihrig) #46017 * (SEMVER-MINOR) add reporters (Moshe Atlow) #45712 v8: * (SEMVER-MINOR) support gc profile (theanarkh) #46255 vm: * (SEMVER-MINOR) expose cachedDataRejected for vm.compileFunction (Anna Henningsen) #46320 PR-URL: https://github.com/nodejs/node/pull/46920/commits
Notable changes: buffer: * (SEMVER-MINOR) add isAscii method (Yagiz Nizipli) #46046 doc,lib,src,test: * rename --test-coverage (Colin Ihrig) #46017 fs: * (SEMVER-MINOR) add statfs() functions (Colin Ihrig) #46358 src,lib: * (SEMVER-MINOR) add constrainedMemory API for process (theanarkh) #46218 test_runner: * add initial code coverage support (Colin Ihrig) #46017 * (SEMVER-MINOR) add reporters (Moshe Atlow) #45712 v8: * (SEMVER-MINOR) support gc profile (theanarkh) #46255 vm: * (SEMVER-MINOR) expose cachedDataRejected for vm.compileFunction (Anna Henningsen) #46320 PR-URL: #46920
Notable changes: buffer: * (SEMVER-MINOR) add isAscii method (Yagiz Nizipli) #46046 doc,lib,src,test: * rename --test-coverage (Colin Ihrig) #46017 fs: * (SEMVER-MINOR) add statfs() functions (Colin Ihrig) #46358 src,lib: * (SEMVER-MINOR) add constrainedMemory API for process (theanarkh) #46218 test_runner: * add initial code coverage support (Colin Ihrig) #46017 * (SEMVER-MINOR) add reporters (Moshe Atlow) #45712 v8: * (SEMVER-MINOR) support gc profile (theanarkh) #46255 vm: * (SEMVER-MINOR) expose cachedDataRejected for vm.compileFunction (Anna Henningsen) #46320 PR-URL: #46920
Notable changes: buffer: * (SEMVER-MINOR) add isAscii method (Yagiz Nizipli) #46046 doc,lib,src,test: * rename --test-coverage (Colin Ihrig) #46017 fs: * (SEMVER-MINOR) add statfs() functions (Colin Ihrig) #46358 src,lib: * (SEMVER-MINOR) add constrainedMemory API for process (theanarkh) #46218 test_runner: * add initial code coverage support (Colin Ihrig) #46017 * (SEMVER-MINOR) add reporters (Moshe Atlow) #45712 v8: * (SEMVER-MINOR) support gc profile (theanarkh) #46255 vm: * (SEMVER-MINOR) expose cachedDataRejected for vm.compileFunction (Anna Henningsen) #46320 PR-URL: #46920
add
constrainedMemory
API for process.cc @anonrig
make -j4 test
(UNIX), orvcbuild test
(Windows) passes