-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Switch to orjson, fallback to stdlib json #6387
base: V3/develop
Are you sure you want to change the base?
Conversation
This makes it unsuitable for Config. Additionally, the support for dataclasses and other types would make it harder to migrate back. |
yikes, well that's a bummer. Is there anything that can be done? or maybe an alternative? |
Nothing can really be done about this in the current Config API. If there's ever a new one, probably, though I don't know if a new one would even have a JSON driver in the first place. You could probably sprinkle some orjson usage in other places in Core, just not anything to do with Config. I don't know if it will really give any significant gains though, I don't think we use JSON outside of Config much. Other than that, I glanced at the diff and I just want to say that there's no point in the |
ah I see
for now I'll just look into this for this PR, if the org thinks that this is really unnecessary than the PR can be closed
perhaps can we use a fallback to |
That is an interesting idea though there are more cases where orjson differs in serialization and that could lead to introducing discrepancies between drivers (and even within the driver in case of the fallback) - for example it serializes UUID types while normal JSON would fail at that point. |
Description of the changes
To optimize performance we should utilize orjson, which is already included in
requirements/base.in
, but unused in Red.I have been using a modified version of Red with orjson (replacing stdlib json, no fallback) without any problem and have noticed performance gains so far, I primarily used
orjson.dumps
andorjson.loads
, while other methods likejson.dump
,json.load
,aiohttp.ClientSession(json_serialize=...)
remained unchanged using stdlib json, also never in audio before.While I don't think a full switch to orjson is necessary, but I think we should give orjson a chance.
Have the changes in this PR been tested?
No