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

Investigate size increase #40

Closed
anonrig opened this issue Jan 10, 2023 · 7 comments · Fixed by #44
Closed

Investigate size increase #40

anonrig opened this issue Jan 10, 2023 · 7 comments · Fixed by #44

Comments

@anonrig
Copy link
Owner

anonrig commented Jan 10, 2023

We should investigate if the size increase is worth the performance gain, that is included in the latest release.

Received a feedback on Twitter

According to Bundlephobia

  • 1.0.0 is 3.6 kb (minified)
  • 1.1.0 is 748 kb (minified)
Fund with Polar
@Uzlopak
Copy link
Contributor

Uzlopak commented Jan 10, 2023

Yeah this is because multibyte Charakters are pre calculated. I made a memory speed trade off. We could calculate at startup. This could slow down the startup time. I could provide a PR if you like.

@anonrig
Copy link
Owner Author

anonrig commented Jan 11, 2023

I think we should do a comparison in performance once again.

@baraeb92
Copy link

Would it be possible for the user to be given the option to opt out on some optimizations?

So user can perhaps decide for themselves on whether 15% faster on some ops is a worthy tradeoff with extra 700k on the app?

That way user who wants to enjoy other features in newer version can do so, without having to pay the 700k toll.

Now my team decided to use the v1.0.0 instead to workaround this problem.

@Uzlopak
Copy link
Contributor

Uzlopak commented Jan 12, 2023

I have somewhere on disk the function with which i generated the array in first place.

@mcollina
Copy link
Collaborator

mcollina commented Feb 4, 2023

I think we should revert that change. 700KB is hardly worth the improvement :(.

Happy to also switch Fastify back to node:query-string.

@anonrig
Copy link
Owner Author

anonrig commented Feb 4, 2023

@Uzlopak Can you revert the changes?

@Fdawgs
Copy link
Contributor

Fdawgs commented Feb 4, 2023

I think we should revert that change. 700KB is hardly worth the improvement :(.

Happy to also switch Fastify back to node:query-string.

I thought disk space usage wasn't a concern for Fastify?

See fastify/skeleton#42

@anonrig anonrig reopened this Nov 30, 2023
@anonrig anonrig closed this as completed Dec 2, 2023
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 a pull request may close this issue.

5 participants