src: multi-isolate support for NodePlatform by addaleax · Pull Request #16700 · nodejs/node
added 2 commits
This splits the task queue used for asynchronous tasks scheduled by V8 in per-isolate queues, so that multiple threads can be supported. PR-URL: ayojs/ayo#89 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Worker threads need an event loop without active libuv handles in order to shut down. One source of handles that was previously not accounted for were delayed V8 tasks; these create timers that would be standing in the way of clearing the event loop. To solve this, keep track of the scheduled tasks in a list and close their timer handles before the corresponding isolate/loop is removed from the platform. It is not clear from the V8 documentation what the expectation is with respect to pending background tasks at the end of the isolate lifetime; however, an alternative approach of executing these scheduled tasks when flushing them led to an infinite loop of tasks scheduling each other; so it seems safe to assume that the behaviour implemented in this patch is at least acceptable. PR-URL: ayojs/ayo#120 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
labels
nodejs-github-bot
added
c++
labels
addaleax
deleted the
platform-multiple-isolates
branch
addaleax added a commit that referenced this pull request
This splits the task queue used for asynchronous tasks scheduled by V8 in per-isolate queues, so that multiple threads can be supported. Original-PR-URL: ayojs/ayo#89 Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com> PR-URL: #16700 Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit that referenced this pull request
Worker threads need an event loop without active libuv handles in order to shut down. One source of handles that was previously not accounted for were delayed V8 tasks; these create timers that would be standing in the way of clearing the event loop. To solve this, keep track of the scheduled tasks in a list and close their timer handles before the corresponding isolate/loop is removed from the platform. It is not clear from the V8 documentation what the expectation is with respect to pending background tasks at the end of the isolate lifetime; however, an alternative approach of executing these scheduled tasks when flushing them led to an infinite loop of tasks scheduling each other; so it seems safe to assume that the behaviour implemented in this patch is at least acceptable. Original-PR-URL: ayojs/ayo#120 Original-Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> PR-URL: #16700 Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request
This splits the task queue used for asynchronous tasks scheduled by V8 in per-isolate queues, so that multiple threads can be supported. Original-PR-URL: ayojs/ayo#89 Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com> PR-URL: #16700 Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Worker threads need an event loop without active libuv handles in order to shut down. One source of handles that was previously not accounted for were delayed V8 tasks; these create timers that would be standing in the way of clearing the event loop. To solve this, keep track of the scheduled tasks in a list and close their timer handles before the corresponding isolate/loop is removed from the platform. It is not clear from the V8 documentation what the expectation is with respect to pending background tasks at the end of the isolate lifetime; however, an alternative approach of executing these scheduled tasks when flushing them led to an infinite loop of tasks scheduling each other; so it seems safe to assume that the behaviour implemented in this patch is at least acceptable. Original-PR-URL: ayojs/ayo#120 Original-Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> PR-URL: #16700 Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request
* update submodule refs for node v9.3.0 * Define "llvm_version" for Node.js build * NODE_MODULE_CONTEXT_AWARE_BUILTIN -> NODE_BUILTIN_MODULE_CONTEXT_AWARE * update NodePlatform to MultiIsolatePlatform * fix linting error * update node ref * REVIEW: Explicitly register builtin modules nodejs/node#16565 * update libcc ref * switch libcc to c62 * REVIEW: Address node api changes - Always start the inspector agent for nodejs/node#17085 - Set the tracing controller for node nodejs/node#15538 - Isolate data creation now requires plaform nodejs/node#16700
zcbenz pushed a commit to electron/electron that referenced this pull request
* update submodule refs for node v9.3.0 * Define "llvm_version" for Node.js build * NODE_MODULE_CONTEXT_AWARE_BUILTIN -> NODE_BUILTIN_MODULE_CONTEXT_AWARE * update NodePlatform to MultiIsolatePlatform * fix linting error * update node ref * REVIEW: Explicitly register builtin modules nodejs/node#16565 * update libcc ref * switch libcc to c62 * REVIEW: Address node api changes - Always start the inspector agent for nodejs/node#17085 - Set the tracing controller for node nodejs/node#15538 - Isolate data creation now requires plaform nodejs/node#16700
alexeykuzmin pushed a commit to electron/electron that referenced this pull request
* update submodule refs for node v9.3.0 * Define "llvm_version" for Node.js build * NODE_MODULE_CONTEXT_AWARE_BUILTIN -> NODE_BUILTIN_MODULE_CONTEXT_AWARE * update NodePlatform to MultiIsolatePlatform * fix linting error * update node ref * REVIEW: Explicitly register builtin modules nodejs/node#16565 * update libcc ref * switch libcc to c62 * REVIEW: Address node api changes - Always start the inspector agent for nodejs/node#17085 - Set the tracing controller for node nodejs/node#15538 - Isolate data creation now requires plaform nodejs/node#16700
alexeykuzmin pushed a commit to electron/electron that referenced this pull request
* update submodule refs for node v9.3.0 * Define "llvm_version" for Node.js build * NODE_MODULE_CONTEXT_AWARE_BUILTIN -> NODE_BUILTIN_MODULE_CONTEXT_AWARE * update NodePlatform to MultiIsolatePlatform * fix linting error * update node ref * REVIEW: Explicitly register builtin modules nodejs/node#16565 * update libcc ref * switch libcc to c62 * REVIEW: Address node api changes - Always start the inspector agent for nodejs/node#17085 - Set the tracing controller for node nodejs/node#15538 - Isolate data creation now requires plaform nodejs/node#16700
zcbenz pushed a commit to electron/electron that referenced this pull request
* update submodule refs for node v9.3.0 * Define "llvm_version" for Node.js build * NODE_MODULE_CONTEXT_AWARE_BUILTIN -> NODE_BUILTIN_MODULE_CONTEXT_AWARE * update NodePlatform to MultiIsolatePlatform * fix linting error * update node ref * REVIEW: Explicitly register builtin modules nodejs/node#16565 * update libcc ref * switch libcc to c62 * REVIEW: Address node api changes - Always start the inspector agent for nodejs/node#17085 - Set the tracing controller for node nodejs/node#15538 - Isolate data creation now requires plaform nodejs/node#16700
zcbenz pushed a commit to electron/electron that referenced this pull request
* update submodule refs for node v9.3.0 * Define "llvm_version" for Node.js build * NODE_MODULE_CONTEXT_AWARE_BUILTIN -> NODE_BUILTIN_MODULE_CONTEXT_AWARE * update NodePlatform to MultiIsolatePlatform * fix linting error * update node ref * REVIEW: Explicitly register builtin modules nodejs/node#16565 * update libcc ref * switch libcc to c62 * REVIEW: Address node api changes - Always start the inspector agent for nodejs/node#17085 - Set the tracing controller for node nodejs/node#15538 - Isolate data creation now requires plaform nodejs/node#16700
sethlu pushed a commit to sethlu/electron that referenced this pull request
* update submodule refs for node v9.3.0 * Define "llvm_version" for Node.js build * NODE_MODULE_CONTEXT_AWARE_BUILTIN -> NODE_BUILTIN_MODULE_CONTEXT_AWARE * update NodePlatform to MultiIsolatePlatform * fix linting error * update node ref * REVIEW: Explicitly register builtin modules nodejs/node#16565 * update libcc ref * switch libcc to c62 * REVIEW: Address node api changes - Always start the inspector agent for nodejs/node#17085 - Set the tracing controller for node nodejs/node#15538 - Isolate data creation now requires plaform nodejs/node#16700
addaleax added a commit to addaleax/node that referenced this pull request
This splits the task queue used for asynchronous tasks scheduled by V8 in per-isolate queues, so that multiple threads can be supported. Original-PR-URL: ayojs/ayo#89 Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com> PR-URL: nodejs#16700 Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit to addaleax/node that referenced this pull request
Worker threads need an event loop without active libuv handles in order to shut down. One source of handles that was previously not accounted for were delayed V8 tasks; these create timers that would be standing in the way of clearing the event loop. To solve this, keep track of the scheduled tasks in a list and close their timer handles before the corresponding isolate/loop is removed from the platform. It is not clear from the V8 documentation what the expectation is with respect to pending background tasks at the end of the isolate lifetime; however, an alternative approach of executing these scheduled tasks when flushing them led to an infinite loop of tasks scheduling each other; so it seems safe to assume that the behaviour implemented in this patch is at least acceptable. Original-PR-URL: ayojs/ayo#120 Original-Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> PR-URL: nodejs#16700 Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request
This splits the task queue used for asynchronous tasks scheduled by V8 in per-isolate queues, so that multiple threads can be supported. Backport-PR-URL: #20901 Original-PR-URL: ayojs/ayo#89 Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com> PR-URL: #16700 Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Worker threads need an event loop without active libuv handles in order to shut down. One source of handles that was previously not accounted for were delayed V8 tasks; these create timers that would be standing in the way of clearing the event loop. To solve this, keep track of the scheduled tasks in a list and close their timer handles before the corresponding isolate/loop is removed from the platform. It is not clear from the V8 documentation what the expectation is with respect to pending background tasks at the end of the isolate lifetime; however, an alternative approach of executing these scheduled tasks when flushing them led to an infinite loop of tasks scheduling each other; so it seems safe to assume that the behaviour implemented in this patch is at least acceptable. Backport-PR-URL: #20901 Original-PR-URL: ayojs/ayo#120 Original-Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> PR-URL: #16700 Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg pushed a commit that referenced this pull request
This splits the task queue used for asynchronous tasks scheduled by V8 in per-isolate queues, so that multiple threads can be supported. Backport-PR-URL: #20901 Original-PR-URL: ayojs/ayo#89 Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com> PR-URL: #16700 Reviewed-By: James M Snell <jasnell@gmail.com>
rvagg pushed a commit that referenced this pull request
Worker threads need an event loop without active libuv handles in order to shut down. One source of handles that was previously not accounted for were delayed V8 tasks; these create timers that would be standing in the way of clearing the event loop. To solve this, keep track of the scheduled tasks in a list and close their timer handles before the corresponding isolate/loop is removed from the platform. It is not clear from the V8 documentation what the expectation is with respect to pending background tasks at the end of the isolate lifetime; however, an alternative approach of executing these scheduled tasks when flushing them led to an infinite loop of tasks scheduling each other; so it seems safe to assume that the behaviour implemented in this patch is at least acceptable. Backport-PR-URL: #20901 Original-PR-URL: ayojs/ayo#120 Original-Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> PR-URL: #16700 Reviewed-By: James M Snell <jasnell@gmail.com>