Kick out of normalizePath if there's nothing to do#44173
Kick out of normalizePath if there's nothing to do#44173amcasey merged 3 commits intomicrosoft:masterfrom
Conversation
|
Note: I tried to reimplement |
|
@typescript-bot perf test this |
|
Heya @amcasey, I've started to run the perf test suite on this PR at d5bfa4b. You can monitor the build here. Update: The results are in! |
|
Note: this PR incorporates #44149. |
src/compiler/path.ts
Outdated
|
|
||
| export function normalizePath(path: string): string { | ||
| path = normalizeSlashes(path); | ||
| path = path.replace(/\/\.\//g, "/"); |
There was a problem hiding this comment.
I could go either way on this replacement. Among paths that require updates, this is commonly enough (1/3 if I had to ballpark it), but the number is dwarfed by the number that require no changes at all (in which case this is a wasted string traversal). I measured it both ways and the difference was less than the margin of error.
src/compiler/path.ts
Outdated
|
|
||
| // check path for these segments: '', '.'. '..' | ||
| const relativePathSegmentRegExp = /(^|\/)\.{0,2}($|\/)/; | ||
| const relativePathSegmentRegExp = /(?:^|\/)\.\.?(?:$|\/)|\/\//; |
There was a problem hiding this comment.
Without this change, the kickout essentially never happens.
There was a problem hiding this comment.
Non-capturing groups are also cheaper than capturing groups, in general, so should be used any time the capture isn't necessary.
|
@amcasey Here they are:Comparison Report - master..44173
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@typescript-bot perf test this |
|
Heya @amcasey, I've started to run the perf test suite on this PR at a9351dd. You can monitor the build here. Update: The results are in! |
|
@amcasey Here they are:Comparison Report - master..44173
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Whoa, that's a lot of time spent in path normalization. I suppose that's not really all that surprising, though, it's a pretty expensive operation and gets run a very large number of times during path resolution. |
...using `relativePathSegmentRegExp`. Bonus: use a regex to handle "/./" to avoid splitting and joining in a common case. When building the compiler, for example, it looks like ~95% of arguments to `normalizePath` do not require any normalization.
|
Force push just merges in #44149 |
|
@typescript-bot perf test this |
|
Heya @amcasey, I've started to run the perf test suite on this PR at 81613f7. You can monitor the build here. Update: The results are in! |
|
@typescript-bot perf test this |
|
Heya @amcasey, I've started to run the perf test suite on this PR at a098b54. You can monitor the build here. Update: The results are in! |
|
@amcasey Here they are:Comparison Report - master..44173
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@amcasey Here they are:Comparison Report - master..44173
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
...using
relativePathSegmentRegExp.Bonus: use a regex to handle "/./" to avoid splitting and joining in a common case.
When building the compiler, for example, it looks like ~95% of arguments to
normalizePathdo not require any normalization.