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

delay costly makeError to when it is needed #218

Merged
merged 1 commit into from
Nov 29, 2020

Conversation

mgilbir
Copy link
Contributor

@mgilbir mgilbir commented Nov 10, 2020

In the context of issue #205, this change makes parsing 4x faster in my tests.

@mgilbir mgilbir mentioned this pull request Nov 10, 2020
@colinhacks
Copy link
Owner

Can you summarize what you did here? Everything was prettified with a shorter printWidth so it's hard to find your changes.

@colinhacks
Copy link
Owner

Ah I just saw you're other comment. Basically your moving the makeError definition out of the parsing function. Makes sense! I'll play around with this.

@mgilbir
Copy link
Contributor Author

mgilbir commented Nov 10, 2020

Agh, sorry! I only saw now how messy the diff got due to applying prettier. I was rushing to send this between meetings and I didn't double check the diff before pushing 😞

I moved the makeError out fo the parsing because in the profiler I saw that the ZodError generation was always present and taking a significant amount of time on JSON blobs that never error. Doing less work on a heavy-hit function was bound to speed things up.

I didn't see any other obvious things in the profiler, so I didn't dig deeper.

@o-alexandrov o-alexandrov mentioned this pull request Nov 17, 2020
8 tasks
@colinhacks colinhacks merged commit ed9ca44 into colinhacks:master Nov 29, 2020
@colinhacks
Copy link
Owner

Merged into Zod v1 and v2-beta. Thanks @mgilbir, great work on this!

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