perf(vite): rework endsWith to direct index #24746

GalacticHypernova · 2023-12-14T09:22:52Z

🔗 Linked issue

❓ Type of change

📖 Documentation (updates to the documentation, readme or JSdoc annotations)
🐞 Bug fix (a non-breaking change that fixes an issue)
👌 Enhancement (improving an existing functionality like performance)
✨ New feature (a non-breaking change that adds functionality)
🧹 Chore (updates to the build process or auxiliary tools and libraries)
⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Similar to #24744, endsWith also has a performance downside when checking single characters. This reworks it to direct index in strings, which inccreases performance by ~40%.
Benchmark results:

const nuxt={
    _ignorePatterns:["abcdefghijklmnopqrstuvwxyz*!","abcdefghijklmnopqrstuvwxyz*!","abcdefghijklmnopqrstuvwxyz*!","abcdefghijklmnopqrstuvwxyz*!","abcdefghijklmnopqrstuvwxyz*!","abcdefghijklmnopqrstuvwxyz*!","abcdefghijklmnopqrstuvwxyz*!","abcdefghijklmnopqrstuvwxyz*!"]
}
console.time("currentSingle")
for (let index = 0; index < 100000; index++) {
    nuxt._ignorePatterns.map(p => p.endsWith('*'))
}
console.timeEnd("currentSingle")
console.time("PRSingle")
for (let index = 0; index < 100000; index++) {
    nuxt._ignorePatterns.map(p=>p[p.length-1]==="*")
}
console.timeEnd("PRSingle")
    console.time("currentM")
for (let index = 0; index < 100000; index++) {
    nuxt._ignorePatterns.map(p => p.endsWith('!*'))
}
console.timeEnd("currentM")
console.time("PRM")
for (let index = 0; index < 100000; index++) {
    nuxt._ignorePatterns.map(p => p[p.length-2] === '!' && p[p.length-1] === '*')
}
console.timeEnd("PRM")

And yes, here too we can make a tradeoff of partial readability for performance if needed.

📝 Checklist

I have linked an issue or discussion.
I have updated the documentation accordingly.

stackblitz · 2023-12-14T09:22:56Z

Run & review this pull request in StackBlitz Codeflow.

GalacticHypernova · 2023-12-14T09:24:40Z

packages/vite/src/plugins/composable-keys.ts

@@ -116,7 +116,8 @@ export const composableKeysPlugin = createUnplugin((options: ComposableKeysOptio
          }

          // TODO: Optimize me (https://github.com/nuxt/framework/pull/8529)
-          const endsWithComma = code.slice(codeIndex + (node as any).start, codeIndex + (node as any).end - 1).trim().endsWith(',')
+          const newCode = code.slice(codeIndex + (node as any).start, codeIndex + (node as any).end - 1).trim()


@danielroe this entire slice/trim operation, is it even needed? Can we not just do this?

code[codeIndex + (node as any).end - 1]===','

I saw that pi left a comment there, is there a possibility codeIndex + (node as any).end - 1 would point to a whitespace?

yes, I think it's possible it will end with whitespace...

Hmm, I'll try to run some tests with this. There's gotta be a way to make it work without it..

you could iterate backwards in a while loop... 🤔

That could be possible, it will at the very least reduce the amount of characters "digested" by trim I suppose which runs both ways (will make some benchmarks for this), but I wonder if there's a way to make it even faster, like if the expression itself will not contain whitespace, in which case last index could be used directly.

For starter optimization, we can use trimEnd which increase performance by ~50%

GalacticHypernova · 2023-12-16T19:41:43Z

@danielroe would you like me to also fix this in tests? Or keep this in packages only?

danielroe · 2023-12-16T19:42:26Z

We can ignore tests. 👌

GalacticHypernova · 2023-12-16T21:05:17Z

👍

GalacticHypernova · 2023-12-17T09:51:35Z

I'm getting some odd results and even odder deviations in my analysis. We should count on offline benchmarks, right? Ones made on a PC rather than on the browser?
Because I find it impossible that one of the benchmarks suggested checking a single character with a simple index is ~33% slower than with endsWith

danielroe · 2023-12-17T10:35:56Z

Benchmarks should be on node and bun, for changes to kit/schema/nuxt build.

I would take perf optimisations with an extreme grain of salt as runtimes frequently optimise newer features in future... In general avoiding sweeping statements.

Ideally reproducible benchmarks should be provided - might be worth using something like codspeed, and adding a comment to remove when things change.

GalacticHypernova · 2023-12-17T13:47:30Z

I had a feeling it was just the online benchmarks faking because on node it is marginally faster.

might be worth using something like codspeed, and adding a comment to remove when things change.

I can do that, but considering endsWith was implemented in ECMA6 and hasn't been optimized since I can say pretty confidently this isn't likely to change anytime soon.

GalacticHypernova · 2024-01-23T16:16:08Z

Well, a little underwhelming that there was only one thing to change that actually makes any sort of difference, but I guess this happens =)

perf(vite): rework endsWith to direct index checking

7112720

github-actions bot added the 3.x label Dec 14, 2023

GalacticHypernova commented Dec 14, 2023

View reviewed changes

GalacticHypernova changed the title ~~perf(vite): rework endsWith to direct index checking~~ perf(vite): rework endsWith to direct index Dec 14, 2023

danielroe marked this pull request as draft December 14, 2023 11:04

GalacticHypernova added 2 commits December 14, 2023 19:42

perf(vite): use trimEnd

347ebc6

Merge branch 'main' into patch-18

bae8777

GalacticHypernova added 5 commits December 17, 2023 15:47

Merge branch 'main' into patch-18

bbe112b

Merge branch 'main' into patch-18

02dda29

chore: revert trimEnd

8ee9adc

Merge branch 'main' into patch-18

074973f

Merge branch 'nuxt:main' into patch-18

b068748

danielroe force-pushed the main branch from f78140e to 4168157 Compare January 9, 2024 09:39

danielroe added the performance label Jan 12, 2024

Merge branch 'main' into patch-18

33dc5a0

GalacticHypernova marked this pull request as ready for review January 23, 2024 16:15

danielroe approved these changes Jan 29, 2024

View reviewed changes

danielroe merged commit 860cfe1 into nuxt:main Jan 29, 2024
33 of 34 checks passed

github-actions bot mentioned this pull request Jan 29, 2024

v3.10.0 #25284

Merged

GalacticHypernova mentioned this pull request Feb 27, 2024

perf: performance overhaul #25771

Closed

9 tasks

GalacticHypernova mentioned this pull request Mar 6, 2024

perf: performance overhaul #26114

Open

9 tasks

GalacticHypernova mentioned this pull request May 14, 2024

perf: use direct array access instead of endsWIth unjs/unstorage#387

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(vite): rework endsWith to direct index #24746

perf(vite): rework endsWith to direct index #24746

GalacticHypernova commented Dec 14, 2023 •

edited

Loading

stackblitz bot commented Dec 14, 2023

GalacticHypernova Dec 14, 2023 •

edited

Loading

danielroe Dec 14, 2023

GalacticHypernova Dec 14, 2023

danielroe Dec 14, 2023

GalacticHypernova Dec 14, 2023 •

edited

Loading

GalacticHypernova Dec 14, 2023

GalacticHypernova commented Dec 16, 2023

danielroe commented Dec 16, 2023

GalacticHypernova commented Dec 16, 2023

GalacticHypernova commented Dec 17, 2023 •

edited

Loading

danielroe commented Dec 17, 2023

GalacticHypernova commented Dec 17, 2023 •

edited

Loading

GalacticHypernova commented Jan 23, 2024

perf(vite): rework endsWith to direct index #24746

perf(vite): rework endsWith to direct index #24746

Conversation

GalacticHypernova commented Dec 14, 2023 • edited Loading

🔗 Linked issue

❓ Type of change

📚 Description

📝 Checklist

stackblitz bot commented Dec 14, 2023

GalacticHypernova Dec 14, 2023 • edited Loading

Choose a reason for hiding this comment

danielroe Dec 14, 2023

Choose a reason for hiding this comment

GalacticHypernova Dec 14, 2023

Choose a reason for hiding this comment

danielroe Dec 14, 2023

Choose a reason for hiding this comment

GalacticHypernova Dec 14, 2023 • edited Loading

Choose a reason for hiding this comment

GalacticHypernova Dec 14, 2023

Choose a reason for hiding this comment

GalacticHypernova commented Dec 16, 2023

danielroe commented Dec 16, 2023

GalacticHypernova commented Dec 16, 2023

GalacticHypernova commented Dec 17, 2023 • edited Loading

danielroe commented Dec 17, 2023

GalacticHypernova commented Dec 17, 2023 • edited Loading

GalacticHypernova commented Jan 23, 2024

GalacticHypernova commented Dec 14, 2023 •

edited

Loading

GalacticHypernova Dec 14, 2023 •

edited

Loading

GalacticHypernova Dec 14, 2023 •

edited

Loading

GalacticHypernova commented Dec 17, 2023 •

edited

Loading

GalacticHypernova commented Dec 17, 2023 •

edited

Loading