-
-
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 #72754
Initial orjson support #72754
Conversation
Redux of home-assistant#32153 Now possible since the following is solved: ijl/orjson#220 (comment)
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 ( |
Still need to store out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good 😎
This will give it a good month and if we notice issues prior to 2022.7 we can adjust.
This was causing the wheels to fail to build. We need to workout why when we don't have release pressure This reverts commit d9d22a9.
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">
Proposed change
Redux of #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:
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)
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: