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 Fleet server performance bottlenecks due to JSON unmarshal #25512

Open
getvictor opened this issue Jan 16, 2025 · 4 comments
Open

Fix Fleet server performance bottlenecks due to JSON unmarshal #25512

getvictor opened this issue Jan 16, 2025 · 4 comments
Labels
~backend Backend-related issue. ~engineering-initiated Engineering-initiated story, such as a bug, refactor, or contributor experience improvement.

Comments

@getvictor
Copy link
Member

Problem: The Go json/v1 API we are using is at least 2X slower at unmarshaling than pretty much any other library. We do a lot of JSON unmarshaling between osquery results, vulnerability processing, etc.

  • json/v2 realistically won't be out for at least a year. We could use it as is, but its API is unstable.
  • I recommend we try https://github.com/goccy/go-json to speed up JSON unmarshal where needed. It is popular (used by 177K open source repos) and API-compatible with json/v1. Once json/v2 is out, we can start using that.

Example performance issue: #24390

Another example from an old vuln processing pprof run:
image.png

@getvictor getvictor added ~backend Backend-related issue. ~engineering-initiated Engineering-initiated story, such as a bug, refactor, or contributor experience improvement. labels Jan 16, 2025
@mostlikelee
Copy link
Contributor

associated risk (thx @mna):

It's interesting to investigate, but the use of unsafe is concerning (IIRC, the json/v2 aims for speedups without using unsafe) and there are 8 reports of panic issues in the first page of issues alone (6 "panic", 1 "exception", 1"segmentation" in the title), and 2 high mem-cpu usage : https://github.com/goccy/go-json/issues

Some have accompaniying PR open for a year and a half. It's... not a good look. Would be good to also verify if they take shortcuts like in fastjson, e.g. to skip bytes assuming a well-formed JSON and not report invalid JSON (we accept user-provided JSON, it's important that our unmarshaler checks its validity).
Some folks have lots of imagination when it comes to topping benchmarks.

@getvictor
Copy link
Member Author

Another extremely popular JSON library is https://github.com/bytedance/sonic and it is also not memory-safe.

If we must have memory safety, then I recommend using https://github.com/go-json-experiment/json

There is another memory-safe alternative, but it doesn't seem very popular:

getvictor added a commit that referenced this issue Feb 18, 2025
For #26218 

Basic Android MDM on/off backend functionality. Manually tested.

The following env vars must be set:
```
FLEET_DEV_ANDROID_ENABLED=1
FLEET_DEV_ANDROID_SERVICE_CREDENTIALS=$(cat credentials.json)
FLEET_DEV_ANDROID_PUBSUB_TOPIC=projects/your-project/topics/your-topic
```

I picked https://github.com/go-json-experiment/json as the JSON library,
which seems like the safest option.
- will become json/v2 at some point
- currently used in production by other companies, like Tailscale
- well-maintained
- Some context here: #25512

Plan for next work:
- refactoring from 1st PR
- add pubsub with device enroll -> spec proxy for fleetdm.com
- come back to this sub-task to add tests and finish TODOs

# Checklist for submitter

- [x] Added/updated automated tests
- [x] Manual QA for all new/changed functionality
@jacobshandling
Copy link
Contributor

In case it might be a decent option, I ran across the simd-json library and this write up about simdjson

@getvictor
Copy link
Member Author

In case it might be a decent option, I ran across the simd-json library and this write up about simdjson

@jacobshandling Thank you for sharing. I took a look at it. It only works on Intel/AMD x86 chips, so it might not be a good fit for our M1 dev machines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
~backend Backend-related issue. ~engineering-initiated Engineering-initiated story, such as a bug, refactor, or contributor experience improvement.
Projects
None yet
Development

No branches or pull requests

3 participants