-
Notifications
You must be signed in to change notification settings - Fork 353
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
Upgrade JSON library #314
Upgrade JSON library #314
Changes from all commits
588eefa
3a44494
a72652d
ea470f7
860db1c
0d8396d
875836f
d7bb830
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,23 +2,17 @@ | |
// The reason is two fold: | ||
// 1. To easily ensure that all calling libraries use the same version (minimize code size) | ||
// 2. To allow us to switch out to eg. serde-json-core more easily | ||
use serde::{Deserialize, Serialize}; | ||
use serde::{de::DeserializeOwned, Serialize}; | ||
use std::any::type_name; | ||
|
||
use crate::encoding::Binary; | ||
use crate::errors::{parse_err, serialize_err, StdResult}; | ||
|
||
pub fn from_slice<'a, T>(value: &'a [u8]) -> StdResult<T> | ||
where | ||
T: Deserialize<'a>, | ||
{ | ||
pub fn from_slice<T: DeserializeOwned>(value: &[u8]) -> StdResult<T> { | ||
serde_json_wasm::from_slice(value).map_err(|e| parse_err(type_name::<T>(), e)) | ||
} | ||
|
||
pub fn from_binary<'a, T>(value: &'a Binary) -> StdResult<T> | ||
where | ||
T: Deserialize<'a>, | ||
{ | ||
pub fn from_binary<T: DeserializeOwned>(value: &Binary) -> StdResult<T> { | ||
from_slice(value.as_slice()) | ||
} | ||
|
||
|
@@ -35,3 +29,84 @@ where | |
{ | ||
to_vec(data).map(Binary) | ||
} | ||
|
||
#[cfg(test)] | ||
mod test { | ||
use super::*; | ||
use serde::Deserialize; | ||
|
||
#[derive(Serialize, Deserialize, Debug, PartialEq)] | ||
#[serde(rename_all = "snake_case")] | ||
enum SomeMsg { | ||
Refund {}, | ||
ReleaseAll { | ||
image: String, | ||
amount: u32, | ||
time: u64, | ||
karma: i32, | ||
}, | ||
Cowsay { | ||
text: String, | ||
}, | ||
} | ||
|
||
#[test] | ||
fn to_vec_works() { | ||
let msg = SomeMsg::Refund {}; | ||
let serialized = to_vec(&msg).unwrap(); | ||
assert_eq!(serialized, br#"{"refund":{}}"#); | ||
|
||
let msg = SomeMsg::ReleaseAll { | ||
image: "foo".to_string(), | ||
amount: 42, | ||
time: 9007199254740999, // Number.MAX_SAFE_INTEGER + 7 | ||
karma: -17, | ||
}; | ||
let serialized = String::from_utf8(to_vec(&msg).unwrap()).unwrap(); | ||
assert_eq!( | ||
serialized, | ||
r#"{"release_all":{"image":"foo","amount":42,"time":9007199254740999,"karma":-17}}"# | ||
); | ||
} | ||
|
||
#[test] | ||
fn from_slice_works() { | ||
let deserialized: SomeMsg = from_slice(br#"{"refund":{}}"#).unwrap(); | ||
assert_eq!(deserialized, SomeMsg::Refund {}); | ||
|
||
let deserialized: SomeMsg = from_slice( | ||
br#"{"release_all":{"image":"foo","amount":42,"time":18446744073709551615,"karma":-17}}"#, | ||
) | ||
.unwrap(); | ||
assert_eq!( | ||
deserialized, | ||
SomeMsg::ReleaseAll { | ||
image: "foo".to_string(), | ||
amount: 42, | ||
time: 18446744073709551615, | ||
karma: -17 | ||
} | ||
); | ||
} | ||
|
||
#[test] | ||
fn to_vec_works_for_special_chars() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. These two |
||
let msg = SomeMsg::Cowsay { | ||
text: "foo\"bar\\\"bla".to_string(), | ||
}; | ||
let serialized = String::from_utf8(to_vec(&msg).unwrap()).unwrap(); | ||
assert_eq!(serialized, r#"{"cowsay":{"text":"foo\"bar\\\"bla"}}"#); | ||
} | ||
|
||
#[test] | ||
fn from_slice_works_for_special_chars() { | ||
let deserialized: SomeMsg = | ||
from_slice(br#"{"cowsay":{"text":"foo\"bar\\\"bla"}}"#).unwrap(); | ||
assert_eq!( | ||
deserialized, | ||
SomeMsg::Cowsay { | ||
text: "foo\"bar\\\"bla".to_string(), | ||
} | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -393,7 +393,7 @@ mod singlepass_test { | |
|
||
let init_used = orig_gas - instance.get_gas(); | ||
println!("init used: {}", init_used); | ||
assert_eq!(init_used, 45568); | ||
assert_eq!(init_used, 65606); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are all quite huge gas increases for the safety check. I'm sure this can be optimized. But like I said, in a point release of serde-json-wasm, maybe in cosmwasm 0.8.1. This is an optimization that doesn't break any APIs |
||
} | ||
|
||
#[test] | ||
|
@@ -417,7 +417,7 @@ mod singlepass_test { | |
|
||
let handle_used = gas_before_handle - instance.get_gas(); | ||
println!("handle used: {}", handle_used); | ||
assert_eq!(handle_used, 63432); | ||
assert_eq!(handle_used, 94480); | ||
} | ||
|
||
#[test] | ||
|
@@ -452,6 +452,6 @@ mod singlepass_test { | |
|
||
let query_used = gas_before_query - instance.get_gas(); | ||
println!("query used: {}", query_used); | ||
assert_eq!(query_used, 23066); | ||
assert_eq!(query_used, 32676); | ||
} | ||
} |
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.
Wow, this increased gas usage a lot! (+/- 28k to +/- 42k)
Maybe it makes sense to have a hot-path when there is no escaped characters inside the string to just do the old copy. We can merge this, but it would be nice to update the json lib before 0.8 (we can do that in a week or so, but it should be possible to do around the same cost when there are no escapes)