From cfa7b54c2552fbaff0029c10d24fa37925e42b02 Mon Sep 17 00:00:00 2001
From: David Frank <david.frank@dfinity.org>
Date: Wed, 21 Aug 2024 18:25:12 +0200
Subject: [PATCH] Optimize the performance of deserializing types containing
 options (#568)

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 | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/rust/candid/src/de.rs b/rust/candid/src/de.rs
index afe5b14a..33bb9529 100644
--- a/rust/candid/src/de.rs
+++ b/rust/candid/src/de.rs
@@ -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;
 
@@ -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())
     }
@@ -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,
@@ -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(),
@@ -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()