Skip to content

Commit

Permalink
Add TypeRegistrationDeserializer and remove BorrowedStr (bevyengi…
Browse files Browse the repository at this point in the history
…ne#7094)

# Objective

This a follow-up to bevyengine#6894, see bevyengine#6894 (comment)

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.
  • Loading branch information
SkiFire13 authored and alradish committed Jan 22, 2023
1 parent 1ccfe7a commit bd3a64d
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 33 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_reflect/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down
67 changes: 51 additions & 16 deletions crates/bevy_reflect/src/serde/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<dyn Reflect>`] containing the deserialized data.
Expand Down Expand Up @@ -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> {
registry: &'a TypeRegistry,
}
Expand All @@ -280,14 +320,9 @@ impl<'a, 'de> Visitor<'de> for UntypedReflectDeserializerVisitor<'a> {
where
A: MapAccess<'de>,
{
let type_name = map
.next_key::<BorrowableCowStr>()?
.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,
Expand Down
28 changes: 12 additions & 16 deletions crates/bevy_scene/src/serde.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
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::{
de::{DeserializeSeed, Error, MapAccess, SeqAccess, Visitor},
ser::SerializeStruct,
Deserialize, Deserializer, Serialize, Serializer,
};
use std::borrow::Cow;
use std::fmt::Formatter;

pub const SCENE_STRUCT: &str = "Scene";
Expand Down Expand Up @@ -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))?,
);
Expand All @@ -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};
Expand Down

0 comments on commit bd3a64d

Please sign in to comment.