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

[target-allocator] Use json-iter to marshal json #1336

Closed
kristinapathak opened this issue Dec 21, 2022 · 2 comments · Fixed by #1410
Closed

[target-allocator] Use json-iter to marshal json #1336

kristinapathak opened this issue Dec 21, 2022 · 2 comments · Fixed by #1410
Labels
area:target-allocator Issues for target-allocator

Comments

@kristinapathak
Copy link
Contributor

In server handling, we currently have two use cases:

  1. marshaling into yaml and then converting yaml to json.
  2. marshaling straight to json using the standard library.

There are json libraries more performant than the json standard library package and provide configuration on the build tag, allowing us to not have to effectively marshal twice for the yaml -> json conversion. I looked into json-iterator, which is imported in other open-telemetry repos and showed good benchmarks. To see if it was the right fit for us, I wrote benchmarks for our server handling functions to see how json-iterator compares to the standard library package:

#1323
benchmarks.txt

In terms of both memory and cpu, json-iterator performed better in all of my tests, especially as objects grew larger.

I'm interested in hearing others' thoughts on this - is this a change people are open to making?

@kristinapathak kristinapathak added the area:target-allocator Issues for target-allocator label Dec 21, 2022
@kristinapathak kristinapathak changed the title [target-allocator] Use json-iterator to marshal json [target-allocator] Use json-iter to marshal json Dec 21, 2022
@jaronoff97
Copy link
Contributor

Given those benchmarks, I am very interested in this change going through!

@kristinapathak
Copy link
Contributor Author

These changes had to be reverted: #1351

The prometheus structs have special MarshalYAML() functions that a json marshaler won't find and use. Specifically the Regex struct has no exported fields, so all data is lost there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:target-allocator Issues for target-allocator
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants