Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimize svg loading, run tsc in lint to check types, don't include extra copies of the fonts in the assets folder #3

Merged
merged 3 commits into from
Jun 1, 2024

Conversation

easrng
Copy link

@easrng easrng commented May 30, 2024

No description provided.

…xtra copies of the fonts in the assets folder
@easrng
Copy link
Author

easrng commented May 30, 2024

i might be stupid

@valadaptive
Copy link
Owner

I'm not sure which part of the code changes is the optimization. Is it the change from Blob#arrayBuffer to FileReader? The aggregation into one <defs> tag?

@easrng
Copy link
Author

easrng commented May 30, 2024

Just a bunch of little things, FileReader is way faster comparatively (especially in Firefox) but they're both pretty fast so it doesn't matter a ton, the fewer DOM operations helped a bit, I tried to make fewer allocations but I didn't actually test that

@valadaptive
Copy link
Owner

I changed the structure of the font loading code back to the previous one while retaining those optimizations. Can you pull that and check if it's still the same speed?

@easrng
Copy link
Author

easrng commented May 30, 2024

You should keep the isFont, it's more idiomatic typescript.

@valadaptive
Copy link
Owner

Added it back in. Is the performance still the same as in your original PR?

@easrng
Copy link
Author

easrng commented May 30, 2024

seems about 0.8s slower at loading 1029722049 start to finish (just averaged over 7 runs each (refreshed before running) cus they're kinda long lol) (... which might not actually be enough to be statistically significant)

@valadaptive
Copy link
Owner

I measured loading 1029722049 and there's a huge amount of variation on the order of seconds, seemingly due to differences in how long it takes to render the parsed SVGs. I'm probably going to merge this as-is

@valadaptive valadaptive merged commit 8a1b559 into valadaptive:main Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants