Skip to content

Commit 802826e

Browse files
authored
refactor: JSON message with less allocations (#16130)
### What does this PR try to resolve? This was found during experimenting `-Zbuild-analysis` with ndjson. From me tracing the code with `cargo expand`, basically there shouldn't have any significant performance difference between `serde(flatten)` and inlining all the fields. Here the differences between them * flatten one calls `Serialize::serialize_map` without fields size hint so cannot pre-allocate Vec with `Vec::with_capacity`, whereas inline case calls `Serialize::serialize_struct` with a known length of fields. * flatten would end up calling `Serializer::serialize_map` and line calls `Serializer::serialize_struct`. And in serde_json serializer `serialize_struct` actually call `serailze_map`. So no difference on serializer side. * There might be some function calls not inlined I like `FlatMapSerializer` but I doubt it is costly than allocation. <details><summary> Here is the `cargo-expand`'d result: </summary> <p> ```rust #[derive(Serialize)] pub struct Foo<D: Serialize> { id: u8, #[serde(flatten)] data: D, } #[derive(Serialize)] struct Bar { a: bool, } // Expand to extern crate serde as _serde; impl<D: Serialize> _serde::Serialize for Foo<D> where D: _serde::Serialize, { fn serialize<__S>( &self, __serializer: __S, ) -> _serde::__private228::Result<__S::Ok, __S::Error> where __S: _serde::Serializer, { let mut __serde_state = _serde::Serializer::serialize_map( __serializer, _serde::__private228::None, )?; _serde::ser::SerializeMap::serialize_entry( &mut __serde_state, "id", &self.id, )?; _serde::Serialize::serialize( &&self.data, _serde::__private228::ser::FlatMapSerializer(&mut __serde_state), )?; _serde::ser::SerializeMap::end(__serde_state) } } impl _serde::Serialize for Bar { fn serialize<__S>( &self, __serializer: __S, ) -> _serde::__private228::Result<__S::Ok, __S::Error> where __S: _serde::Serializer, { let mut __serde_state = _serde::Serializer::serialize_struct( __serializer, "Bar", false as usize + 1, )?; _serde::ser::SerializeStruct::serialize_field( &mut __serde_state, "a", &self.a, )?; _serde::ser::SerializeStruct::end(__serde_state) } } ``` ```rust #[derive(Serialize)] pub struct Foo<D: Serialize> { id: u8, a: bool, } // Expand to impl<D: Serialize> _serde::Serialize for Foo<D> { fn serialize<__S>( &self, __serializer: __S, ) -> _serde::__private228::Result<__S::Ok, __S::Error> where __S: _serde::Serializer, { let mut __serde_state = _serde::Serializer::serialize_struct( __serializer, "Foo", false as usize + 1 + 1, )?; _serde::ser::SerializeStruct::serialize_field( &mut __serde_state, "id", &self.id, )?; _serde::ser::SerializeStruct::serialize_field( &mut __serde_state, "a", &self.a, )?; _serde::ser::SerializeStruct::end(__serde_state) } } ``` </p> </details> ### How to test and review this PR? CI passing. One change is that `reason` will no longer be the first field. We could activate the `preserve_order` feature in `serde_json` if we want, though that may get more perf loess than the gain. See the benchmark result in <#16130 (comment)>
2 parents 36bc1d1 + 4ec3bb3 commit 802826e

File tree

1 file changed

+12
-5
lines changed

1 file changed

+12
-5
lines changed

src/cargo/util/machine_message.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
33
use cargo_util_schemas::core::PackageIdSpec;
44
use serde::Serialize;
55
use serde::ser;
6-
use serde_json::{json, value::RawValue};
6+
use serde_json::value::RawValue;
77

88
use crate::core::Target;
99
use crate::core::compiler::{CompilationSection, CompileMode};
@@ -12,10 +12,17 @@ pub trait Message: ser::Serialize {
1212
fn reason(&self) -> &str;
1313

1414
fn to_json_string(&self) -> String {
15-
let json = serde_json::to_string(self).unwrap();
16-
assert!(json.starts_with("{\""));
17-
let reason = json!(self.reason());
18-
format!("{{\"reason\":{},{}", reason, &json[1..])
15+
#[derive(Serialize)]
16+
struct WithReason<'a, S: Serialize> {
17+
reason: &'a str,
18+
#[serde(flatten)]
19+
msg: &'a S,
20+
}
21+
let with_reason = WithReason {
22+
reason: self.reason(),
23+
msg: &self,
24+
};
25+
serde_json::to_string(&with_reason).unwrap()
1926
}
2027
}
2128

0 commit comments

Comments
 (0)