Nice performance boost for handling boolean values #36
Conversation
src/lib.rs
Outdated
| } else if let Ok(key) = key.str() { | ||
| let key = key.to_string().map_err(debug_py_err)?; | ||
| map.serialize_key(&key)?; | ||
| match key { | ||
| Cow::Borrowed("true") => map.serialize_key(&true)?, |
There was a problem hiding this comment.
I don't think this branch is needed -- a boolean key true and a string key "true" serialize exactly the same in JSON.
There was a problem hiding this comment.
Just to be clear, you mean we could get rid of the match for Cow::Borrowed("true") and Cow::Borrowed("false") and simply always use the _ case?
There was a problem hiding this comment.
Seems to work in the unit tests at least. 😅 Let me push that change.
|
Which of the graphs are you looking at? To me it looks like the 100 dicts array was substantially faster before (left) than after (right).
|
|
Oh. You're right. 😞 Maybe it was just wishful thinking on my side. |
|
I don't think we will be able to make reasonable progress on performance just by trying code changes. I would recommend getting a reliable |
|
Duly noted. I'm on a Mac and it feels like the Linux support is better when it comes to profiling Rust code. There are still options of course like @carols10cents' write-up here or this experimental profiler for mac. |
|
Closing this PR as it was misguided. Sorry for that. |
We can serialize to bool directly without using
extract.This gives us a nice performance boost as can be seen in the new plots.
The same trick can probably be applied elsewhere.
Good places to look for such improvements are the remaining
extractcalls and the macro handling for casts.