src: refactor `SplitString` in util#48491

anonrig · 2023-06-18T16:25:01Z

benchmark code

#include<string_view> #include<algorithm>// uses string::find_first_of std::vector<std::string> split(const std::string& str, const std::string& delims = ""){std::vector<std::string> output; //output.reserve(str.length() / 4);size_t first = 0; while (first < str.size()){constauto second = str.find_first_of(delims, first); if (first != second){output.emplace_back(str.substr(first, second-first))} if (second == std::string::npos) break; first = second + 1} return output} std::vector<std::string> splitStd(const std::string& str, const std::string& delims = ""){std::vector<std::string> output; auto first = std::cbegin(str); while (first != std::cend(str)){constauto second = std::find_first_of(first, std::cend(str), std::cbegin(delims), std::cend(delims)); if (first != second) output.emplace_back(first, second); if (second == std::cend(str)) break; first = std::next(second)} return output} std::vector<std::string> splitPtr(const std::string& str, const std::string& delims = ""){std::vector<std::string> output; // output.reserve(str.size() / 2);for (auto first = str.data(), second = str.data(), last = first + str.size(); second != last && first != last; first = second + 1){second = std::find_first_of(first, last, std::cbegin(delims), std::cend(delims)); if (first != second) output.emplace_back(first, second)} return output} std::vector<std::string_view> splitSV(std::string_view strv, std::string_view delims = ""){std::vector<std::string_view> output; size_t first = 0; while (first < strv.size()){constauto second = strv.find_first_of(delims, first); if (first != second) output.emplace_back(strv.substr(first, second-first)); if (second == std::string_view::npos) break; first = second + 1} return output} std::vector<std::string_view> splitSVStd(std::string_view strv, std::string_view delims = ""){std::vector<std::string_view> output; //output.reserve(strv.length() / 4);auto first = strv.begin(); while (first != strv.end()){constauto second = std::find_first_of(first, std::cend(strv), std::cbegin(delims), std::cend(delims)); //std::cout << first << ", " << second << '\n';if (first != second){output.emplace_back(strv.substr(std::distance(strv.begin(), first), std::distance(first, second)))} if (second == strv.end()) break; first = std::next(second)} return output} std::vector<std::string_view> splitSVPtr(std::string_view str, std::string_view delims = ""){std::vector<std::string_view> output; //output.reserve(str.size() / 2);for (auto first = str.data(), second = str.data(), last = first + str.size(); second != last && first != last; first = second + 1){second = std::find_first_of(first, last, std::cbegin(delims), std::cend(delims)); if (first != second) output.emplace_back(first, second - first)} return output} const std::string_view LoremIpsumStrv{"Lorem ipsum dolor sit amet, consectetur adipiscing elit,""sed do eiusmod tempor incididuntsuperlongwordsuper ut labore et dolore magna aliqua. Ut enim ad minim veniam, ""quis nostrud exercitation ullamco laboris nisi ut aliquipsuperlongword ex ea commodo consequat. Duis aute""irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.""Excepteur sint occaecat cupidatatsuperlongword non proident, sunt in culpa qui officia deserunt mollit anim id est laborum." }; staticvoidStringSplit(benchmark::State& state){std::string str{LoremIpsumStrv }; // Code inside this loop is measured repeatedlyfor (auto _ : state){auto v = split(str); benchmark::DoNotOptimize(v)} } // Register the function as a benchmarkBENCHMARK(StringSplit); staticvoidStringSplitStd(benchmark::State& state){std::string str{LoremIpsumStrv }; // Code inside this loop is measured repeatedlyfor (auto _ : state){auto v = splitStd(str); benchmark::DoNotOptimize(v)} } // Register the function as a benchmarkBENCHMARK(StringSplitStd); staticvoidStringSplitPtr(benchmark::State& state){std::string str{LoremIpsumStrv }; // Code inside this loop is measured repeatedlyfor (auto _ : state){auto v = splitPtr(str); benchmark::DoNotOptimize(v)} } // Register the function as a benchmarkBENCHMARK(StringSplitPtr); staticvoidStringViewSplit(benchmark::State& state){for (auto _ : state){auto v = splitSV(LoremIpsumStrv); benchmark::DoNotOptimize(v)} } BENCHMARK(StringViewSplit); staticvoidStringViewSplitStd(benchmark::State& state){for (auto _ : state){auto v = splitSVStd(LoremIpsumStrv); benchmark::DoNotOptimize(v)} } BENCHMARK(StringViewSplitStd); staticvoidStringViewSplitPtr(benchmark::State& state){for (auto _ : state){auto v = splitSVPtr(LoremIpsumStrv); benchmark::DoNotOptimize(v)} } BENCHMARK(StringViewSplitPtr);

nodejs-github-bot · 2023-06-18T16:25:06Z

Review requested:

@nodejs/security-wg

targos · 2023-06-18T16:46:48Z

Please at least post the benchmark results in the PR. Your link may become dead in the future

src/permission/fs_permission.cc

nodejs-github-bot · 2023-06-21T16:54:10Z

CI: https://ci.nodejs.org/job/node-test-pull-request/52324/

nodejs-github-bot · 2023-06-22T16:08:04Z

CI: https://ci.nodejs.org/job/node-test-pull-request/52350/

src/node_options.cc

src/node_v8_platform-inl.h

src/permission/fs_permission.cc

anonrig · 2023-06-22T19:27:42Z

cc @nodejs/cpp-reviewers

nodejs-github-bot · 2023-06-22T20:06:49Z

CI: https://ci.nodejs.org/job/node-test-pull-request/52363/

nodejs-github-bot · 2023-06-22T22:23:30Z

Landed in 640a791

PR-URL: #48491 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>

PR-URL: nodejs#48491 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>

PR-URL: #48491 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>

The use of string_view and subsequent copying to a string was supposed to be a minor optimization in 640a7918, however, since 413c16e, no string splitting occurs anymore. Therefore, we can simply pass around some references instead of using string_view or copying strings. Refs: nodejs#48491 Refs: nodejs#49047

The use of string_view and subsequent copying to a string was supposed to be a minor optimization in 640a7918, however, since 413c16e, no string splitting occurs anymore. Therefore, we can simply pass around some references instead of using string_view or copying strings. Refs: #48491 Refs: #49047 PR-URL: #50662 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>

The use of string_view and subsequent copying to a string was supposed to be a minor optimization in 640a7918, however, since 413c16e, no string splitting occurs anymore. Therefore, we can simply pass around some references instead of using string_view or copying strings. Refs: nodejs#48491 Refs: nodejs#49047 PR-URL: nodejs#50662 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>

The use of string_view and subsequent copying to a string was supposed to be a minor optimization in 640a7918, however, since 413c16e, no string splitting occurs anymore. Therefore, we can simply pass around some references instead of using string_view or copying strings. Refs: #48491 Refs: #49047 PR-URL: #50662 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jun 18, 2023

anonrig added the performance Issues and PRs related to the performance of Node.js. label Jun 18, 2023

RafaelGSS reviewed Jun 20, 2023
View reviewed changes

src/permission/fs_permission.ccShow resolvedHide resolved

jasnell approved these changes Jun 20, 2023
View reviewed changes

anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 21, 2023

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 21, 2023

src: refactor SplitString in util
b53ae22

anonrig removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 22, 2023