tools: add ArrayPrototypeConcat to the list of primordials to avoid#44445
tools: add ArrayPrototypeConcat to the list of primordials to avoid#44445nodejs-github-bot merged 2 commits intonodejs:mainfrom
ArrayPrototypeConcat to the list of primordials to avoid#44445Conversation
|
Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1266/ Results |
a60b429 to
b67284e
Compare
|
/cc @nodejs/tsc for reviews |
There was a problem hiding this comment.
This reminds me that we once had a JS function dedicated to pushing items into an array so that C++ can call that instead of Object::Set (which was slower), and I got rid of that by adding an Array::New() that takes an array of Local<Value> from C++. I wonder if it'd be faster now in cases like getActiveResourcesInfo() if we simply assemble the array from C++ using Array::New()......
|
Landed in ca2ec90 |
This change reduces the number of calls that were crossing the JS-C++ boundary to 1 and also removes the need for calling Array::New() multiple times internally and ArrayPrototypeConcat-ing the results later on, thus improving performance. Refs: nodejs#44445 (review) Signed-off-by: Darshan Sen <[email protected]>
PR-URL: #44445 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
This change reduces the number of calls that were crossing the JS-C++ boundary to 1 and also removes the need for calling Array::New() multiple times internally and ArrayPrototypeConcat-ing the results later on, thus improving performance. Refs: #44445 (review) Signed-off-by: Darshan Sen <[email protected]> PR-URL: #46014 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
|
As mentioned in the v19.4.0 proposal (#46061 (comment)), this seems to be causing unexpected behavior in some modules. So, I'm including the flag More details of the error can be found on the proposal CITGM. |
|
Feel free to remove it when revisited. |
|
Removed the labels as #46108 fixed the unexpected behavior. |
PR-URL: nodejs#44445 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
This change reduces the number of calls that were crossing the JS-C++ boundary to 1 and also removes the need for calling Array::New() multiple times internally and ArrayPrototypeConcat-ing the results later on, thus improving performance. Refs: nodejs#44445 (review) Signed-off-by: Darshan Sen <[email protected]> PR-URL: nodejs#46014 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
PR-URL: #44445 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
This change reduces the number of calls that were crossing the JS-C++ boundary to 1 and also removes the need for calling Array::New() multiple times internally and ArrayPrototypeConcat-ing the results later on, thus improving performance. Refs: #44445 (review) Signed-off-by: Darshan Sen <[email protected]> PR-URL: #46014 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
PR-URL: #44445 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
This change reduces the number of calls that were crossing the JS-C++ boundary to 1 and also removes the need for calling Array::New() multiple times internally and ArrayPrototypeConcat-ing the results later on, thus improving performance. Refs: #44445 (review) Signed-off-by: Darshan Sen <[email protected]> PR-URL: #46014 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
This change reduces the number of calls that were crossing the JS-C++ boundary to 1 and also removes the need for calling Array::New() multiple times internally and ArrayPrototypeConcat-ing the results later on, thus improving performance. Refs: #44445 (review) Signed-off-by: Darshan Sen <[email protected]> PR-URL: #46014 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
Refs: #44366 (comment)