perf(vite): rework endsWith to direct index

[GalacticHypernova]

🔗 Linked issue

nuxt/framework#8529

❓ 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]

Run & review this pull request in StackBlitz Codeflow.

[GalacticHypernova]

@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?

[danielroe]

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

[GalacticHypernova]

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

[danielroe]

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

[GalacticHypernova]

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.

[GalacticHypernova]

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

[GalacticHypernova]

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

[danielroe]

We can ignore tests. 👌

[GalacticHypernova]

👍

[GalacticHypernova]

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]

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]

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]

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