Upgrade JSON library #314
Upgrade JSON library #314
Conversation
Both visit_borrowed_str and visit_string forward to visit_str
|
This is done as far as I can see |
There was a problem hiding this comment.
Looks good, please merge and tag.
I created CosmWasm/serde-json-wasm#16 to be done either in 0.8.0 or 0.8.1. No rush on it, but don't want to forget it.
| @@ -339,8 +339,9 @@ mod singlepass_tests { | |||
| // Gas consumtion is relatively small | |||
| // Note: the exact gas usage depends on the Rust version used to compile WASM, | |||
| // which we only fix when using cosmwasm-opt, not integration tests. | |||
| assert!(gas_used > 26000, "{}", gas_used); | |||
| assert!(gas_used < 30000, "{}", gas_used); | |||
| let expected = 42000; // +/- 20% | |||
There was a problem hiding this comment.
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)
| } | ||
|
|
||
| #[test] | ||
| fn to_vec_works_for_special_chars() { |
| @@ -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.
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
Closes #110
Closes #312