Skip to content

Swap json.Unmarshal with jsoniter.Unmarshal #315

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

Closed
wants to merge 2 commits into from

Conversation

sarahzinger
Copy link

I was looking into some OOMs for this plugin as part of my on call shift and noticed that we are using the standard json package. My understanding is that in grafana/grafana we have switched to using jsoniter in several places because it is at least in theory more performant.

I did a benchmark test here and noticed that jsoniter uses significantly less memory (in these test examples anyway) for Unmarshal...but oddly performed worse for Marshal.

So I made a little package to use whichever one is more performant at the time. But maybe this is overkill and we'd rather just use jsoniter directly and no need for the various performance tests. Btw I used cursor on this and am not a memory usage or bench testing expert so definitely let me know if I'm missing anything!

a note: it might be interesting to add more realistic test cases here if you have them to see if it is indeed more performant... feel free to take over this pr and do with it whatever calls to you and/or disregard but use it for inspiration. I think after this it might be interesting to start putting size limits but that feels worth discussing as a team potentially with product as I'm not sure what a reasonable limit here would be. But if you want a code example of reading size limits I have linked one in the ticket below:

relates to: https://github.com/grafana/data-sources/issues/50

@sarahzinger sarahzinger requested a review from a team as a code owner March 7, 2025 18:52
@sarahzinger sarahzinger closed this Mar 7, 2025
@github-project-automation github-project-automation bot moved this to Complete in OSS Big Tent Mar 7, 2025
@sarahzinger
Copy link
Author

Whoops sorry I was misreading our own profiling here. I believe that these json unmarshal calls are actually quite small and the issue is from within google's library not our own, so this won't actually make any memory improvements for this plugin. Please ignore sorry about that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

1 participant