From ae4d0f794a808b88f8b83da7eb96ca87b3d751d4 Mon Sep 17 00:00:00 2001 From: Yan Chen <48968912+chenyan-dfinity@users.noreply.github.com> Date: Thu, 11 Apr 2024 09:59:33 -0700 Subject: [PATCH] use btreemap for type_map and release (#542) * use btreemap for type_map * more btreemap * fix * disable unroll * bump and changelog --- Cargo.lock | 2 +- Changelog.md | 9 +++++++++ rust/candid/Cargo.toml | 2 +- rust/candid/src/ser.rs | 19 ++++++++++--------- rust/candid/src/types/internal.rs | 24 ++++++++++++------------ rust/candid/tests/serde.rs | 5 +++-- 6 files changed, 36 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d8ee7276..f21af487 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -234,7 +234,7 @@ checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" [[package]] name = "candid" -version = "0.10.6" +version = "0.10.7" dependencies = [ "anyhow", "bincode", diff --git a/Changelog.md b/Changelog.md index 72544ca8..e8dcf97c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,15 @@ # Changelog +## 2024-04-11 + +### Candid 0.10.7 -- 0.10.5 + +* Switch `HashMap` to `BTreeMap` in serialization and `T::ty()`. This leads to around 20% perf improvement for serializing complicated types. +* Disable memoization for unrolled types in serialization to save cycle cost. In some cases, type table can get slightly larger, but it's worth the trade off. +* Fix bug in `text_size` +* Fix decoding cost calculation overflow + ## 2024-02-27 ### Candid 0.10.4 diff --git a/rust/candid/Cargo.toml b/rust/candid/Cargo.toml index 733fecbd..93949bae 100644 --- a/rust/candid/Cargo.toml +++ b/rust/candid/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "candid" -version = "0.10.6" +version = "0.10.7" edition = "2021" rust-version.workspace = true authors = ["DFINITY Team"] diff --git a/rust/candid/src/ser.rs b/rust/candid/src/ser.rs index 8da313d0..002c6717 100644 --- a/rust/candid/src/ser.rs +++ b/rust/candid/src/ser.rs @@ -7,7 +7,7 @@ use super::types::value::IDLValue; use super::types::{internal::Opcode, Field, Type, TypeEnv, TypeInner}; use byteorder::{LittleEndian, WriteBytesExt}; use leb128::write::{signed as sleb128_encode, unsigned as leb128_encode}; -use std::collections::HashMap; +use std::collections::BTreeMap; use std::io; use std::vec::Vec; @@ -224,7 +224,7 @@ impl<'a> types::Compound for Compound<'a> { #[derive(Default)] pub struct TypeSerialize { type_table: Vec>, - type_map: HashMap, + type_map: BTreeMap, env: TypeEnv, args: Vec, result: Vec, @@ -235,7 +235,7 @@ impl TypeSerialize { pub fn new() -> Self { TypeSerialize { type_table: Vec::new(), - type_map: HashMap::new(), + type_map: BTreeMap::new(), env: TypeEnv::new(), args: Vec::new(), result: Vec::new(), @@ -262,12 +262,13 @@ impl TypeSerialize { // from the type table. // Someone should implement Pottier's O(nlogn) algorithm // http://gallium.inria.fr/~fpottier/publis/gauthier-fpottier-icfp04.pdf - let unrolled = types::internal::unroll(t); - if let Some(idx) = self.type_map.get(&unrolled) { - let idx = *idx; - self.type_map.insert(t.clone(), idx); - return Ok(()); - } + // Disable this "optimization", as unroll is expensive and has to be called on every recursion. + // let unrolled = types::internal::unroll(t); + // if let Some(idx) = self.type_map.get(&unrolled) { + // let idx = *idx; + // self.type_map.insert(t.clone(), idx); + // return Ok(()); + // } let idx = self.type_table.len(); self.type_map.insert(t.clone(), idx as i32); diff --git a/rust/candid/src/types/internal.rs b/rust/candid/src/types/internal.rs index 63608c95..c6d2b983 100644 --- a/rust/candid/src/types/internal.rs +++ b/rust/candid/src/types/internal.rs @@ -1,13 +1,13 @@ use super::CandidType; use crate::idl_hash; use std::cell::RefCell; -use std::collections::HashMap; +use std::collections::BTreeMap; use std::fmt; // This is a re-implementation of std::any::TypeId to get rid of 'static constraint. // The current TypeId doesn't consider lifetime while computing the hash, which is // totally fine for Candid type, as we don't care about lifetime at all. -#[derive(Debug, PartialEq, Eq, Hash, Clone)] +#[derive(Debug, PartialEq, Eq, Hash, Clone, PartialOrd, Ord)] pub struct TypeId { id: usize, pub name: &'static str, @@ -33,8 +33,8 @@ pub fn type_of(_: &T) -> TypeId { #[derive(Default)] struct TypeName { - type_name: HashMap, - name_index: HashMap, + type_name: BTreeMap, + name_index: BTreeMap, } impl TypeName { fn get(&mut self, id: &TypeId) -> String { @@ -164,10 +164,10 @@ impl TypeContainer { } } -#[derive(Debug, PartialEq, Hash, Eq, Clone)] +#[derive(Debug, PartialEq, Hash, Eq, Clone, PartialOrd, Ord)] pub struct Type(pub std::rc::Rc); -#[derive(Debug, PartialEq, Hash, Eq, Clone)] +#[derive(Debug, PartialEq, Hash, Eq, Clone, PartialOrd, Ord)] pub enum TypeInner { Null, Bool, @@ -382,7 +382,7 @@ pub fn text_size(t: &Type, limit: i32) -> Result { } } -#[derive(Debug, Eq, Clone)] +#[derive(Debug, Eq, Clone, PartialOrd, Ord)] pub enum Label { Id(u32), Named(String), @@ -423,7 +423,7 @@ impl std::hash::Hash for Label { pub type SharedLabel = std::rc::Rc