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.
This commit is contained in:
Giacomo Stevanato 2023-01-09 21:57:14 +00:00
parent 9be47e3328
commit 871c80c103
3 changed files with 64 additions and 33 deletions

View File

@ -29,7 +29,7 @@ downcast-rs = "1.2"
parking_lot = "0.12.1" parking_lot = "0.12.1"
thiserror = "1.0" thiserror = "1.0"
once_cell = "1.11" once_cell = "1.11"
serde = { version = "1", features = ["derive"] } serde = "1"
smallvec = { version = "1.6", features = ["serde", "union", "const_generics"], optional = true } smallvec = { version = "1.6", features = ["serde", "union", "const_generics"], optional = true }
glam = { version = "0.22", features = ["serde"], optional = true } glam = { version = "0.22", features = ["serde"], optional = true }

View File

@ -12,7 +12,6 @@ use serde::de::{
}; };
use serde::Deserialize; use serde::Deserialize;
use std::any::TypeId; use std::any::TypeId;
use std::borrow::Cow;
use std::fmt; use std::fmt;
use std::fmt::{Debug, Display, Formatter}; use std::fmt::{Debug, Display, Formatter};
use std::slice::Iter; 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. /// A general purpose deserializer for reflected types.
/// ///
/// This will return a [`Box<dyn Reflect>`] containing the deserialized data. /// This will return a [`Box<dyn Reflect>`] 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<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
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<E>(self, type_name: &str) -> Result<Self::Value, E>
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> { struct UntypedReflectDeserializerVisitor<'a> {
registry: &'a TypeRegistry, registry: &'a TypeRegistry,
} }
@ -280,14 +320,9 @@ impl<'a, 'de> Visitor<'de> for UntypedReflectDeserializerVisitor<'a> {
where where
A: MapAccess<'de>, A: MapAccess<'de>,
{ {
let type_name = map let registration = map
.next_key::<BorrowableCowStr>()? .next_key_seed(TypeRegistrationDeserializer::new(self.registry))?
.ok_or_else(|| Error::invalid_length(0, &"at least one entry"))? .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 value = map.next_value_seed(TypedReflectDeserializer { let value = map.next_value_seed(TypedReflectDeserializer {
registration, registration,
registry: self.registry, registry: self.registry,

View File

@ -1,7 +1,10 @@
use crate::{DynamicEntity, DynamicScene}; use crate::{DynamicEntity, DynamicScene};
use anyhow::Result; use anyhow::Result;
use bevy_reflect::serde::{TypedReflectDeserializer, TypedReflectSerializer}; 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 bevy_utils::HashSet;
use serde::ser::SerializeMap; use serde::ser::SerializeMap;
use serde::{ use serde::{
@ -9,7 +12,6 @@ use serde::{
ser::SerializeStruct, ser::SerializeStruct,
Deserialize, Deserializer, Serialize, Serializer, Deserialize, Deserializer, Serialize, Serializer,
}; };
use std::borrow::Cow;
use std::fmt::Formatter; use std::fmt::Formatter;
pub const SCENE_STRUCT: &str = "Scene"; 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 added = HashSet::new();
let mut components = Vec::new(); let mut components = Vec::new();
while let Some(BorrowableCowStr(key)) = map.next_key()? { while let Some(registration) =
if !added.insert(key.clone()) { map.next_key_seed(TypeRegistrationDeserializer::new(self.registry))?
return Err(Error::custom(format!("duplicate component: `{key}`"))); {
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( components.push(
map.next_value_seed(TypedReflectDeserializer::new(registration, self.registry))?, 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)] #[cfg(test)]
mod tests { mod tests {
use crate::serde::{SceneDeserializer, SceneSerializer}; use crate::serde::{SceneDeserializer, SceneSerializer};