From 5cda42682aa650b59f8eeccc46ad295a3b1b73bb Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Mon, 9 Jan 2023 21:57:14 +0000 Subject: [PATCH] Add `TypeRegistrationDeserializer` and remove `BorrowedStr` (#7094) # Objective This a follow-up to #6894, see https://github.com/bevyengine/bevy/pull/6894#discussion_r1045203113 The goal is to avoid cloning any string when getting a `&TypeRegistration` corresponding to a string which is being deserialized. As a bonus code duplication is also reduced. ## Solution The manual deserialization of a string and lookup into the type registry has been moved into a separate `TypeRegistrationDeserializer` type, which implements `DeserializeSeed` with a `Visitor` that accepts any string with `visit_str`, even ones that may not live longer than that function call. `BorrowedStr` has been removed since it's no longer used. --- ## Changelog - The type `TypeRegistrationDeserializer` has been added, which simplifies getting a `&TypeRegistration` while deserializing a string. --- crates/bevy_reflect/Cargo.toml | 2 +- crates/bevy_reflect/src/serde/de.rs | 67 ++++++++++++++++++++++------- crates/bevy_scene/src/serde.rs | 28 ++++++------ 3 files changed, 64 insertions(+), 33 deletions(-) diff --git a/crates/bevy_reflect/Cargo.toml b/crates/bevy_reflect/Cargo.toml index c7d54e9964837..f74889c5ab06b 100644 --- a/crates/bevy_reflect/Cargo.toml +++ b/crates/bevy_reflect/Cargo.toml @@ -29,7 +29,7 @@ downcast-rs = "1.2" parking_lot = "0.12.1" thiserror = "1.0" once_cell = "1.11" -serde = { version = "1", features = ["derive"] } +serde = "1" smallvec = { version = "1.6", features = ["serde", "union", "const_generics"], optional = true } glam = { version = "0.22", features = ["serde"], optional = true } diff --git a/crates/bevy_reflect/src/serde/de.rs b/crates/bevy_reflect/src/serde/de.rs index 91021d33d68b8..799ccf33550e2 100644 --- a/crates/bevy_reflect/src/serde/de.rs +++ b/crates/bevy_reflect/src/serde/de.rs @@ -12,7 +12,6 @@ use serde::de::{ }; use serde::Deserialize; use std::any::TypeId; -use std::borrow::Cow; use std::fmt; use std::fmt::{Debug, Display, Formatter}; use std::slice::Iter; @@ -211,13 +210,6 @@ impl<'de> Visitor<'de> for U32Visitor { } } -/// Helper struct for deserializing strings without allocating (when possible). -/// -/// Based on [this comment](https://github.com/bevyengine/bevy/pull/6894#discussion_r1045069010). -#[derive(Deserialize)] -#[serde(transparent)] -struct BorrowableCowStr<'a>(#[serde(borrow)] Cow<'a, str>); - /// A general purpose deserializer for reflected types. /// /// This will return a [`Box`] containing the deserialized data. @@ -265,6 +257,54 @@ impl<'a, 'de> DeserializeSeed<'de> for UntypedReflectDeserializer<'a> { } } +/// A deserializer for type registrations. +/// +/// This will return a [`&TypeRegistration`] corresponding to the given type. +/// This deserializer expects a string containing the _full_ [type name] of the +/// type to find the `TypeRegistration` of. +/// +/// [`&TypeRegistration`]: crate::TypeRegistration +/// [type name]: std::any::type_name +pub struct TypeRegistrationDeserializer<'a> { + registry: &'a TypeRegistry, +} + +impl<'a> TypeRegistrationDeserializer<'a> { + pub fn new(registry: &'a TypeRegistry) -> Self { + Self { registry } + } +} + +impl<'a, 'de> DeserializeSeed<'de> for TypeRegistrationDeserializer<'a> { + type Value = &'a TypeRegistration; + + fn deserialize(self, deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct TypeRegistrationVisitor<'a>(&'a TypeRegistry); + + impl<'de, 'a> Visitor<'de> for TypeRegistrationVisitor<'a> { + type Value = &'a TypeRegistration; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("string containing `type` entry for the reflected value") + } + + fn visit_str(self, type_name: &str) -> Result + where + E: Error, + { + self.0.get_with_name(type_name).ok_or_else(|| { + Error::custom(format_args!("No registration found for `{type_name}`")) + }) + } + } + + deserializer.deserialize_str(TypeRegistrationVisitor(self.registry)) + } +} + struct UntypedReflectDeserializerVisitor<'a> { registry: &'a TypeRegistry, } @@ -280,14 +320,9 @@ impl<'a, 'de> Visitor<'de> for UntypedReflectDeserializerVisitor<'a> { where A: MapAccess<'de>, { - let type_name = map - .next_key::()? - .ok_or_else(|| Error::invalid_length(0, &"at least one entry"))? - .0; - - let registration = self.registry.get_with_name(&type_name).ok_or_else(|| { - Error::custom(format_args!("No registration found for `{type_name}`")) - })?; + let registration = map + .next_key_seed(TypeRegistrationDeserializer::new(self.registry))? + .ok_or_else(|| Error::invalid_length(0, &"at least one entry"))?; let value = map.next_value_seed(TypedReflectDeserializer { registration, registry: self.registry, diff --git a/crates/bevy_scene/src/serde.rs b/crates/bevy_scene/src/serde.rs index e1896d988c206..873015105d1f3 100644 --- a/crates/bevy_scene/src/serde.rs +++ b/crates/bevy_scene/src/serde.rs @@ -1,7 +1,10 @@ use crate::{DynamicEntity, DynamicScene}; use anyhow::Result; use bevy_reflect::serde::{TypedReflectDeserializer, TypedReflectSerializer}; -use bevy_reflect::{serde::UntypedReflectDeserializer, Reflect, TypeRegistry, TypeRegistryArc}; +use bevy_reflect::{ + serde::{TypeRegistrationDeserializer, UntypedReflectDeserializer}, + Reflect, TypeRegistry, TypeRegistryArc, +}; use bevy_utils::HashSet; use serde::ser::SerializeMap; use serde::{ @@ -9,7 +12,6 @@ use serde::{ ser::SerializeStruct, Deserialize, Deserializer, Serialize, Serializer, }; -use std::borrow::Cow; use std::fmt::Formatter; pub const SCENE_STRUCT: &str = "Scene"; @@ -353,15 +355,16 @@ impl<'a, 'de> Visitor<'de> for ComponentVisitor<'a> { { let mut added = HashSet::new(); let mut components = Vec::new(); - while let Some(BorrowableCowStr(key)) = map.next_key()? { - if !added.insert(key.clone()) { - return Err(Error::custom(format!("duplicate component: `{key}`"))); + while let Some(registration) = + map.next_key_seed(TypeRegistrationDeserializer::new(self.registry))? + { + if !added.insert(registration.type_id()) { + return Err(Error::custom(format_args!( + "duplicate component: `{}`", + registration.type_name() + ))); } - let registration = self - .registry - .get_with_name(&key) - .ok_or_else(|| Error::custom(format!("no registration found for `{key}`")))?; components.push( map.next_value_seed(TypedReflectDeserializer::new(registration, self.registry))?, ); @@ -385,13 +388,6 @@ impl<'a, 'de> Visitor<'de> for ComponentVisitor<'a> { } } -/// Helper struct for deserializing strings without allocating (when possible). -/// -/// Based on [this comment](https://github.com/bevyengine/bevy/pull/6894#discussion_r1045069010). -#[derive(Deserialize)] -#[serde(transparent)] -struct BorrowableCowStr<'a>(#[serde(borrow)] Cow<'a, str>); - #[cfg(test)] mod tests { use crate::serde::{SceneDeserializer, SceneSerializer};