Skip to content

Commit

Permalink
Optimize the performance of deserializing types containing options (#568
Browse files Browse the repository at this point in the history
)

The recoverable_visit_some method needs to clone the Deserializer in order to backtrack in case of errors. This cloning is a very expensive operation which this PR optimizes.

1. Wrap the TypeEnv in an Rc. Since the TypeEnv is a heavy object and is barely updated after the Deserializer is constructed, it makes sense to keep a single instance of it so cloning becomes cheap.
2. Make the happy path in recoverable_visit_some as cheap as possible by recursing on self instead of the clone. In case of an error, we reset the state from the clone.
  • Loading branch information
frankdavid authored Aug 21, 2024
1 parent f324a16 commit cfa7b54
Showing 1 changed file with 12 additions and 12 deletions.
24 changes: 12 additions & 12 deletions rust/candid/src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use binread::BinRead;
use byteorder::{LittleEndian, ReadBytesExt};
use serde::de::{self, Visitor};
use std::fmt::Write;
use std::{collections::VecDeque, io::Cursor, mem::replace};
use std::{collections::VecDeque, io::Cursor, mem::replace, rc::Rc};

const MAX_TYPE_LEN: i32 = 500;

Expand Down Expand Up @@ -59,7 +59,7 @@ impl<'de> IDLDeserialize<'de> {
env: &TypeEnv,
expected_type: &Type,
) -> Result<crate::types::value::IDLValue> {
self.de.table.merge(env)?;
Rc::make_mut(&mut self.de.table).merge(env)?;
self.de.is_untyped = true;
self.deserialize_with_type(expected_type.clone())
}
Expand Down Expand Up @@ -298,7 +298,7 @@ macro_rules! check_recursion {
#[derive(Clone)]
struct Deserializer<'de> {
input: Cursor<&'de [u8]>,
table: TypeEnv,
table: Rc<TypeEnv>,
types: VecDeque<(usize, Type)>,
wire_type: Type,
expect_type: Type,
Expand All @@ -322,7 +322,7 @@ impl<'de> Deserializer<'de> {
let (env, types) = header.to_types()?;
Ok(Deserializer {
input: reader,
table: env,
table: env.into(),
types: types.into_iter().enumerate().collect(),
wire_type: TypeInner::Unknown.into(),
expect_type: TypeInner::Unknown.into(),
Expand Down Expand Up @@ -603,15 +603,15 @@ impl<'de> Deserializer<'de> {
}
// This is safe, because the visitor either impl Copy or is zero sized
let v = unsafe { std::ptr::read(&visitor) };
let mut self_clone = self.clone();
match v.visit_some(&mut self_clone) {
Ok(v) => {
*self = self_clone;
Ok(v)
}
let self_clone = self.clone();
match v.visit_some(&mut *self) {
Ok(v) => Ok(v),
Err(Error::Subtype(_)) => {
// Remember the backtracking cost
self.config = self_clone.config;
*self = Self {
// Remember the backtracking cost
config: self.config.clone(),
..self_clone
};
self.add_cost(10)?;
self.deserialize_ignored_any(serde::de::IgnoredAny)?;
visitor.visit_none()
Expand Down

0 comments on commit cfa7b54

Please sign in to comment.