Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
v12.18.2 proposal#34077
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
v12.18.2 proposal #34077
Uh oh!
There was an error while loading. Please reload this page.
Conversation
BethGriggs commented Jun 26, 2020 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Original commit message: [objects] Compact and shrink script_list So far creating scripts always grew the script_list without ever reusing cleared slots or shrinking. While this is probably not a problem with script_list in practice, this is still a memory leak. Fix this leak by using WeakArrayList::Append instead of AddToEnd. Append adds to the end of the array, but potentially compacts and shrinks the list as well. Other WeakArrayLists can use this method as well, as long as they are not using indices into this array. Bug: v8:10031 Change-Id: If743c4cc3f8d67ab735522f0ded038b2fb43e437 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967385 Commit-Queue: Dominik Inführ <[email protected]> Reviewed-by: Ulan Degenbaev <[email protected]> Cr-Commit-Position: refs/heads/master@{#65640} Refs: v8/v8@fb26d0b PR-URL: #33573 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Beth Griggs <[email protected]>
Use a symbol on the bindings object to store the public resource object, rather than a `v8::Global` Persistent. This has several advantages: - It’s harder to inadvertently create memory leaks this way. The garbage collector sees the `AsyncWrap` → resource link like a regular JS property, and can collect the objects as a group, even if the resource object should happen to point back to the `AsyncWrap` object. - This will make it easier in the future to use `owner_symbol` for this purpose, which is generally the direction we should be moving the `async_hooks` API into (i.e. using more public objects instead of letting internal wires stick out). PR-URL: #31745 Backport-PR-URL: #33962 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]>
f5ca930 to 412c618Comparenodejs-github-bot commented Jun 26, 2020 • edited by BethGriggs
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by BethGriggs
Uh oh!
There was an error while loading. Please reload this page.
BethGriggs commented Jun 27, 2020
mhdawson left a comment
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.
LGTM
BethGriggs commented Jun 30, 2020
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker-nobuild/858/ |
2020-06-30, Version 12.18.2 'Erbium' (LTS), @BethGriggs
Notable changes
PrototypeUsers::AddAsyncWrapresource (Anna Henningsen) #31745Commits
97a3f7b702] - deps: V8: backport fb26d0bb1835 (Matheus Marchini) #3357330b0339061] - src: use symbol to storeAsyncWrapresource (Anna Henningsen) #31745From nodejs/Release#494 (comment):