[fix] huge parser performance boost for reading big attribute values

[MathiasWP]

The bug

I worked on a svelte-kit application with some very large SVG's, and they made it almost impossible to work on the application (because of slow development time).

I have created a repo with one of the SVG's if you want to see it yourself: https://github.com/MathiasWP/svelte-attribute-bottleneck

Note that i created the repo with the old svelte-template because i wanted to ensure this wasn't a Vite bug.

The solution

I found out that the method read_attribute_value + read_sequence in src/compiler/parse/state/tag.ts was the big bottleneck. It has a while loop that loops n = template.length amount of times (in the test cases i have added this was 1.7 million times).

On every loop a RegEx check was ran with the parser. This RegEx call was suboptimal, so i changed the code from using parser.match_regex to using parser.match with characters that mirror the RegEx call.

Changing from using RegEx to a strict string equality check massively improved the performance:

With regex: ~30s With new setup: ~260ms

This is a 99% shave-off in execution time 🎉

Before submitting the PR, please make sure you do the following

• [ ] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• [X] Prefix your PR title with [feat], [fix], [chore], or [docs].
• [X] This message body should clearly illustrate what problems it solves.
• [X] Ideally, include a test that fails without this PR but passes with it.

Tests

• [X] Run the tests with npm test and lint the project with npm run lint

[benmccann]

wow, that's a pretty nice speedup!

I didn't look super closely at the test, but I wasn't sure exactly what it was testing or how it worked at first glance. Is it a test that would fail without this change? Performance improvements are hard to add tests for...

[MathiasWP]

wow, that's a pretty nice speedup!

I didn't look super closely at the test, but I wasn't sure exactly what it was testing or how it worked at first glance. Is it a test that would fail without this change? Performance improvements are hard to add tests for...

@benmccann i added three tests, one with an svg, one with an input and one with a textarea. In all of the tests they have an attribute that is crazy long (it's originally a base64 encoded string of one of the svg's from the initial problem).

The tests will fail without the proposed changes because they will trigger a timeout error. They took around 30s to run. You can see this if you replace my optimization with the current solution 👍🏼

[Conduitry]

I'm not wild about adding huge amounts of text to the repo to test this. Can the tests be written so that the large amounts of text that are parsed are generated at runtime?

In either case, whatever tests we keep, the .solo on them needs to be removed.

[MathiasWP]

@Conduitry i changed the test so it generates the value on runtime, removed .solo and removed 2/3 tests.

[MathiasWP]

I have spotted that the array I've created has a lot of similarities with an array in the invalid_attribute_name_character in the src/runtime/internal/ssr.ts file.

Maybe the same optimization could be applied here to increase SSR perfomance, and also refactor so that the same array is used in both files? @tanhauhau @benmccann

[tanhauhau]

Maybe the same optimization could be applied here to increase SSR perfomance

let's keep the scope of this PR as is, can try this in a new PR if it improves the performance for SSR

[MathiasWP]

I did a mistake and changed the "/>" to a "/" in the list of invalid attribute characters. Fixed this, so the list should match the RegEx again now.