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

fix: memory leak in large files with lots of errors #180

Merged
merged 2 commits into from
Mar 27, 2023

Conversation

erunion
Copy link
Member

@erunion erunion commented Mar 27, 2023

🧰 Changes

This resolves a memory leak in how we handle very large invalid API definitions with a lot of errors. For context, with this spec running validation right now either takes upwards of minutes or will fully crash with a memory leak.

Upon investigation I discovered that our @readme/better-ajv-errors library uses @babel/code-frame which when given a large file and a large amount of errors runs code highlighting against this large file for every error it's generating a code frame for. For example: with this spec, which has upwards of 1,000 errors, only rendering the first 20 took 27 seconds with this code highlighting work. Without? 2 seconds.

227815166-82ec7136-f147-4abd-9e5a-0baa7a735c0d

I've updated @readme/better-ajv-errors in readmeio/better-ajv-errors#166 to no longer run code highlighting on these files as we're always handling them as JSON and highlighting everything as green does adds nothing of value to me.

With that fix however, this parsing library would still hit memory leaks:

with colorizing

To resolve this I've decided to cap the amount of errors we render out for very large specs. For this I am only rendering the first 20 error message if the spec in question has a stringified length of 5,000,000 (this amounts to a little less than half the size of the large spec above in question). Validation now works for these situations:

Screen Shot 2023-03-26 at 6 04 48 PM

Additionally since we are not showing the full amount of errors for these situations I've added a new message to the bottom of the errors we show informing the user that they have an additional amount of errors that they will see once they re-run validaiton.

Screen Shot 2023-03-26 at 6 32 33 PM

I don't really love this error handling work because showing errors for this large spec still takes 10seconds and we're not even showing them everything, but thankfully(?) this situation is only going to be experienced by folks with very large API definitions that have a lot of errors.

@erunion erunion added bug Something isn't working refactor Issues about tackling technical debt labels Mar 27, 2023
@erunion
Copy link
Member Author

erunion commented Mar 27, 2023

I'm going to ignore the browser CI failures here because these browser tests have always been a bit flaky.

@erunion erunion merged commit 391b4ef into main Mar 27, 2023
@erunion erunion deleted the fix/memory-leak-on-large-files branch March 27, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working refactor Issues about tackling technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants