From d6931d687e3c952b7e2067edea44be59d7ba5e81 Mon Sep 17 00:00:00 2001 From: David Frank Date: Wed, 21 Aug 2024 10:24:59 +0000 Subject: [PATCH] Optimize the performance of deserializing types containing options 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. --- rust/candid/src/de.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/rust/candid/src/de.rs b/rust/candid/src/de.rs index afe5b14a..bd175025 100644 --- a/rust/candid/src/de.rs +++ b/rust/candid/src/de.rs @@ -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; @@ -59,7 +60,7 @@ impl<'de> IDLDeserialize<'de> { env: &TypeEnv, expected_type: &Type, ) -> Result { - 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()) } @@ -298,7 +299,7 @@ macro_rules! check_recursion { #[derive(Clone)] struct Deserializer<'de> { input: Cursor<&'de [u8]>, - table: TypeEnv, + table: Rc, types: VecDeque<(usize, Type)>, wire_type: Type, expect_type: Type, @@ -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(), @@ -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()