-
Notifications
You must be signed in to change notification settings - Fork 579
Add a RawValue type #355
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
Comments
Yeah, this sounds useful. Ideally we could get some of the building blocks for this sort of thing (custom 'built-in' types) integrated into serde itself, in time. |
I need this for a project I'm working on, and would be happy to give it shot following the same approach as #416. Is this something that you'd accept a PR for? |
Yes I would like a PR for this. When you say you need this feature, is it because deserializing to serde_json::Value is too slow in your project and you are expecting this to have better performance, or some other reason? |
Yeah, I've got an http web server that's backed by a postgres database which stores JSONB values, and would like to shuffle data back and forth (and embed into other payloads) with as few allocations as possible. |
As a first step, please put together a short benchmark that you trust as reasonably representative of the shuffling data back and forth piece of your application. Ideally it would depend on serde / serde_derive / serde_json but nothing else -- so no postgres or http stuff. Focus on the data structures so that they have the same ratio and pattern of raw vs meaningful content as your real use case. Write it using serde_json::Value for now, and then the goal will be to swap in a raw value once it's implemented and observe a speedup. |
Wrote a small microbenchmark here: https://gist.github.com/srijs/7fe21b43c2cf5d2ceeb593ae4591c6f5 Please let me know if this is what you had in mind! |
👍 looks good, let's make it work. |
I released one possible implementation of this in serde_json_experimental 1.0.29-rc1. Here is the documentation. I would be interested in hearing people's experience with this API before committing to it. [dependencies]
serde_json_experimental = { version = "1.0.29-rc1", features = ["raw_value"] } |
From the linked PR:
|
@dtolnay will this be behind a feature flag when released in |
I would like for this to be feature gated because it adds code to the critical path of applications even that do not use RawValue. In what ways do you see crates needing to add support for RawValue where the feature gate would be an obstacle? |
That makes sense to me from a performance standpoint. I'm not super familiar with how feature flags interact in cargo, but my concern is that in order to e.g. add an On a different note, I've been giving the experimental release a shot in my project, and it's been working well so far. The only thing I ran into that wasn't easy to solve was converting a |
No; Cargo features are globally additive. |
As @shepmaster wrote, I believe Cargo handles that situation correctly. If the postgres crate enables the feature and provides that impl, any crate that depends on postgres can use the impl. Regarding impl RawValue {
pub fn from_string(json: String) -> Result<Box<Self>> {
{
let borrowed = ::from_str::<&Self>(&json)?;
if borrowed.json.len() < json.len() {
return Ok(borrowed.to_owned());
}
}
Ok(Self::from_owned(json.into_boxed_str()))
}
} |
Thanks, I wasn't quite sure, but it sounds like cargo will do the right thing 👍 Yeah, I'd like to avoid the extra malloc + memcpy if possible. Haven't had a chanche to benchmark it yet, just wanted to prevent it from becoming a bottleneck for larger documents in the first place. |
Released in 1.0.29: https://docs.rs/serde_json/1.0/serde_json/value/struct.RawValue.html |
It would be helpful to have a type similar to Go's
json.RawMessage
that is not tokenized during deserialization, but rather its raw contents stored as aVec<u8>
or&'de [u8]
.The following pseudo-code demonstrates the idea.
The main advantage of this is would be to have 'lazily-deserialized' values.
The text was updated successfully, but these errors were encountered: