Skip to content

Commit

Permalink
Optimize the performance of deserializing types containing options
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 committed Aug 21, 2024
1 parent 34b4eb0 commit d6931d6
Showing 1 changed file with 10 additions and 7 deletions.
17 changes: 10 additions & 7 deletions rust/candid/src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use byteorder::{LittleEndian, ReadBytesExt};
use serde::de::{self, Visitor};
use std::fmt::Write;
use std::{collections::VecDeque, io::Cursor, mem::replace};
use std::rc::Rc;

const MAX_TYPE_LEN: i32 = 500;

Expand Down Expand Up @@ -59,7 +60,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 +299,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 +323,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 @@ -604,14 +605,16 @@ 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) {
match v.visit_some(&mut *self) {
Ok(v) => {
*self = self_clone;
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 d6931d6

Please sign in to comment.