path: improve posixSplitPath performance #3034
Conversation
|
LGTM |
|
meh, test-stringbytes-external on arm failed...as well as test-child-process-emfile on freebsd. I haven't seen that one fail before. Is that one known to be flaky at all? |
|
@evanlucas They both are flaky, so don't worry about it :-) |
| @@ -408,7 +408,13 @@ var posix = {}; | |||
|
|
|||
|
|
|||
| function posixSplitPath(filename) { | |||
| return splitPathRe.exec(filename).slice(1); | |||
| const matches = splitPathRe.exec(filename); | |||
| var len = matches.length - 1; | |||
There was a problem hiding this comment.
If there is no match, then matches will be null.
There was a problem hiding this comment.
That would have been the case before too, though. I'm not sure if it's possible to get no match with the regexp that's currently being used ...
|
Just curious - why not |
|
@alexlamsl Nice catch. I swear I had tried that before and it was a tiny bit slower than manually doing it, but that is not correct. It looks like shift is a little faster. Thanks! |
|
CI looks happy (minus |
|
I still have my doubts about |
|
@thefourtheye let me see if I can convince you... Part 1 So this pattern will always match. |
Instead of slicing the first element off of the matches, shift and then return. This improves performance of the following path functions: - basename: 18-20% - extname: 60-70% - dirname: 18-20% - parse: 20-25% PR-URL: nodejs#3034 Reviewed-By: Brian White <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
|
Thanks! Landed in b50e89e |
Instead of slicing the first element off of the matches, shift and then return. This improves performance of the following path functions: - basename: 18-20% - extname: 60-70% - dirname: 18-20% - parse: 20-25% PR-URL: #3034 Reviewed-By: Brian White <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Instead of slicing the first element off of the matches, manually build
the array. This improves performance of the following path.posix functions: