-
Notifications
You must be signed in to change notification settings - Fork 17
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
avoid recursion #31
base: main
Are you sure you want to change the base?
avoid recursion #31
Conversation
As far as I understand recursion is mainly dangerous because you risk overflowing the stack. However I think thats unlikely when diffing JSON, unless your JSON has huge levels of nestings. What do you think? Have you seen any problems caused by the recursion or have you benchmarked it? |
I have not seen any overflow, so I think we're safe here. What I can observe on production: when we use this change + #30, then the server consumes much less memory. |
Hm I wouldn't expect recursion to impact memory usage that much. Have you tried it with just #30? |
No I have not. This would take some time. Do you need that? |
Yes. I don't think we refactor the code without a good reason as the change might introduce bugs. |
OK I did the experiment. |
Alright. Thanks for testing it! I'll actually review the code then :) |
|
I wont say no to that. Go for it! 😅 |
@davidpdrsn The pipeline is green now. |
@davidpdrsn ok for you to merge this? |
When comparing deep json structures, the recursion can be dangerous.