nodejs
fs: optimize `fs.cpSync` js calls
https://github.com/nodejs/node/pull/53612 is required to do a benchmark
A different approach to https://github.com/nodejs/node/pull/53541
cc @nodejs/performance @nodejs/cpp-reviewers @billywhizz @jasnell @nodejs/fs
Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1573/
Small win on happy path, and probably high win on error path:
confidence improvement accuracy (*) (**) (***)
fs/bench-cpSync.js n=1 *** 21.96 % ±6.85% ±9.14% ±11.97%
fs/bench-cpSync.js n=100 *** 14.91 % ±2.86% ±3.81% ±4.97%
fs/bench-cpSync.js n=10000 *** 9.74 % ±1.28% ±1.71% ±2.23%
cc @nodejs/tsc @nodejs/fs appreciate your input on which path to take.
previous pr:
- is a sem-ver major
- introduces 2 different implementations (js only and cpp)
- had limitations due to
std::filesystem::copy
this pr:
- is not a sem-ver major
- optimizes all flags passed to cpSync
- optimizes error cases as same as the previous pr
- has a lot more optimization to do
Windows compilation is failing:
C:\workspace\node-compile-windows-debug\node\deps\v8\src\base\bits.h(477,31): warning C4146: unary minus operator applied to unsigned type, result still unsigned [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\mksnapshot.vcxproj]
embedded-file-writer.cc
mksnapshot.cc
embedded-empty.cc
platform-embedded-file-writer-base.cc
platform-embedded-file-writer-generic.cc
platform-embedded-file-writer-win.cc
platform-embedded-file-writer-aix.cc
platform-embedded-file-writer-mac.cc
snapshot-empty.cc
static-roots-gen.cc
Creating library ..\..\out\Debug\mksnapshot.lib and object ..\..\out\Debug\mksnapshot.exp
mksnapshot.vcxproj -> ..\..\out\Debug\\mksnapshot.exe
generating: "..\..\out\Debug\obj\v8_snapshot\/snapshot.cc" "..\..\out\Debug\obj\v8_snapshot\/embedded.S"
Assembling ..\..\out\Debug\obj\v8_snapshot\\embedded.S...
setup-isolate-deserialize.cc
C:\workspace\node-compile-windows-debug\node\deps\v8\src\base\bits.h(477,31): warning C4146: unary minus operator applied to unsigned type, result still unsigned [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_snapshot.vcxproj]
snapshot.cc
C:\workspace\node-compile-windows-debug\node\deps\v8\src\base\bits.h(477,31): warning C4146: unary minus operator applied to unsigned type, result still unsigned [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_snapshot.vcxproj]
v8_snapshot.vcxproj -> ..\..\out\Debug\lib\v8_snapshot.lib
Building locally, I got this:
C:\Users\vinic\Desktop\Projects\node\src\node_file.cc(3036,15): error C2664: 'void node::Environment::ThrowUVException(int,const char *,const char *,const char *,const char *)': cannot convert argument 4 from
'const std::filesystem::path::value_type *' to 'const char *' [C:\Users\vinic\Desktop\Projects\node\libnode.vcxproj]
Building locally, I got this:
C:\Users\vinic\Desktop\Projects\node\src\node_file.cc(3036,15): error C2664: 'void node::Environment::ThrowUVException(int,const char *,const char *,const char *,const char *)': cannot convert argument 4 from 'const std::filesystem::path::value_type *' to 'const char *' [C:\Users\vinic\Desktop\Projects\node\libnode.vcxproj]
This is also in the log from the CI. Based on the error message, this looks very similar to what we had in https://github.com/nodejs/node/pull/53600 because std::filesystem::path::value_type is different on POSIX and Windows.
This is also in the log from the CI. Based on the error message, this looks very similar to what we had in https://github.com/nodejs/node/pull/53600 because std::filesystem::path::value_type is different on POSIX and Windows.
It seems I forgot a src_path.c_str() there. Thank you! @StefanStojanovic @H4ad