-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
Initial orjson support take 2 #72847
Conversation
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( |
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( |
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( |
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( |
Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration ( |
I have my fare share of experience with orjson with a project that is moving a lot of data round (talk million of records). Nice job on this! |
FYI: If you're interested in another approach (if creating the wheels keeps an issue), you may want to look into another really great project: mashumaro Despite the bit weird name it is really great and stable and basically it speeds up the creation of json from dataclasses. |
No changes, rebase for conflicts |
Still need to work out problem building wheels -- Redux of home-assistant#72754 / home-assistant#32153 Now possible since the following is solved: ijl/orjson#220 (comment) This implements orjson where we use our default encoder. This does not implement orjson where `ExtendedJSONEncoder` is used as these areas tend to be called far less frequently. If its desired, this could be done in a followup, but it seemed like a case of diminishing returns (except maybe for large diagnostics files, or traces, but those are not expected to be downloaded frequently). Areas where this makes a perceptible difference: - Anything that subscribes to entities (Initial subscribe_entities payload) - Initial download of registries on first connection / restore - History queries - Saving states to the database - Large logbook queries - Anything that subscribes to events (appdaemon) Cavets: orjson supports serializing dataclasses natively (and much faster) which eliminates the need to implement `as_dict` in many places when the data is already in a dataclass. This works well as long as all the data in the dataclass can also be serialized. I audited all places where we have an `as_dict` for a dataclass and found only backups needs to be adjusted (support for `Path` needed to be added for backups). I was a little bit worried about `SensorExtraStoredData` with `Decimal` but it all seems to work out from since it converts it before it gets to the json encoding cc @dgomes If it turns out to be a problem we can disable this with option |= [orjson.OPT_PASSTHROUGH_DATACLASS](https://github.com/ijl/orjson#opt_passthrough_dataclass) and it will fallback to `as_dict` Its quite impressive for history queries <img width="1271" alt="Screen_Shot_2022-05-30_at_23_46_30" src="https://user-images.githubusercontent.com/663432/171145699-661ad9db-d91d-4b2d-9c1a-9d7866c03a73.png">
Bumped to 3.7.2 |
Thanks for the link. This one is less about dataclasses and more about overall json performance. Every UI that pulls significant data from the websocket is noticeably snappier with |
We can apply this to the http view response json as well as thats a bottleneck as well based on the profiles provided by @Mariusthvdb |
Switching the views made a huge difference in the response time of |
I added some additional tweaks to smooth out some more of the hotspots in the profile @Mariusthvdb sent |
Both @Mariusthvdb and @gieljnssns 's profiles show that |
yes, and I wonder if here_travel_time shows up in those profilers, because it is a long term trouble maker, and currently gone worse. Author is on it, but I thought it might be good to know related to this issue: #73632 (comment) |
It will only show up in a |
Replaced by #73849
Still need to work out problem building wheels which may be solved once #73830 and #73628 merge, but I'm keeping this PR fresh since its what I am running on my production in case anyone else wants to do testing.
--
Redux of #72754 / #32153 Now possible since the following is solved:
ijl/orjson#220 (comment)
This implements orjson where we use our default encoder. This does not implement orjson where
ExtendedJSONEncoder
is used as these areas tend to be called far less frequently. If its desired, this could be done in a followup, but it seemed like a case of diminishing returns (except maybe for large diagnostics files, or traces, but those are not expected to be downloaded frequently).Areas where this makes a perceptible difference:
.json
value_json
Cavets:
orjson supports serializing dataclasses natively (and much faster) which
eliminates the need to implement
as_dict
in many placeswhen the data is already in a dataclass. This works
well as long as all the data in the dataclass can also
be serialized. I audited all places where we have an
as_dict
for a dataclass and found only backups needs to be adjusted (support for
Path
needed to be added for backups). I was a little bit worried aboutSensorExtraStoredData
withDecimal
but it all seems to work out from since it converts it before it gets to the json encoding cc @dgomesIf it turns out to be a problem we can disable this
with option |= orjson.OPT_PASSTHROUGH_DATACLASS and it
will fallback to
as_dict
Its quite impressive for history queries
![Screen_Shot_2022-05-30_at_23_46_30](https://user-images.githubusercontent.com/663432/171145699-661ad9db-d91d-4b2d-9c1a-9d7866c03a73.png)
Breaking change
Proposed change
Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: