fix(node): reduce deepReadDirSync runtime complexity #7910
Conversation
| @@ -27,10 +27,11 @@ export function deepReadDirSync(targetDir: string): string[] { | |||
| const itemAbsPath = path.join(currentDirAbsPath, itemName); | |||
|
|
|||
| if (fs.statSync(itemAbsPath).isDirectory()) { | |||
| return [...absPaths, ...deepReadCurrentDir(itemAbsPath)]; | |||
| return absPaths.concat(deepReadCurrentDir(itemAbsPath)); | |||
There was a problem hiding this comment.
Depending on how we transpile this, it might already be transpiled to something like [].concat(abspaths, [deppread]), doesn't hurt to omit the empty array though
There was a problem hiding this comment.
We transpile to es6, so the spread operator is kept.
| @@ -27,10 +27,11 @@ export function deepReadDirSync(targetDir: string): string[] { | |||
| const itemAbsPath = path.join(currentDirAbsPath, itemName); | |||
|
|
|||
| if (fs.statSync(itemAbsPath).isDirectory()) { | |||
| return [...absPaths, ...deepReadCurrentDir(itemAbsPath)]; | |||
| return absPaths.concat(deepReadCurrentDir(itemAbsPath)); | |||
There was a problem hiding this comment.
We transpile to es6, so the spread operator is kept.
|
Ah dang @AbhiPrasad, maybe something for v8 of SDK to remove then. |
|
@JonasBa which rules are you talking about? I've never heard of eslint doing performance stuff |
Thanks for sharing! Definitely an interesting idea worth exploring. |
|
Sounds like we need a blog post @JonasBa 😄 |
Maybe let the project ripe a little bit more 🙃 But I agree, would surely be interesting if it turns out as promising as expected |
|
@jeengbe @AbhiPrasad I guess this would fall into a similar set of rules as https://eslint.org/docs/latest/rules/no-await-in-loop |
I'm experimenting with some eslint rules to detect runtime performance issues and part of my testing was to run it on sentry-javascript and sentry codebase. There are currently only two rules, one for detecting On^2 in reduce callbacks and a rule for detecting unnecessary array spread operators (which the codebase doesn't seem to use much of anyways).
The reduce rule found one instance where the reducer runtime complexity is exponential (due to the accumulator being spread on each iteration). The fix is to treat the accumulator as mutable and avoid shallow copies inside each iteration.
(gh is not very helpful with reviewer suggestions so tagging @lforst and @AbhiPrasad, but feel free to reassign)