Skip to content

Commit

Permalink
use btreemap for type_map and release (#542)
Browse files Browse the repository at this point in the history
* use btreemap for type_map

* more btreemap

* fix

* disable unroll

* bump and changelog
  • Loading branch information
chenyan-dfinity committed Apr 11, 2024
1 parent b66a88d commit ae4d0f7
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 25 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion rust/candid/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "candid"
version = "0.10.6"
version = "0.10.7"
edition = "2021"
rust-version.workspace = true
authors = ["DFINITY Team"]
Expand Down
19 changes: 10 additions & 9 deletions rust/candid/src/ser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -224,7 +224,7 @@ impl<'a> types::Compound for Compound<'a> {
#[derive(Default)]
pub struct TypeSerialize {
type_table: Vec<Vec<u8>>,
type_map: HashMap<Type, i32>,
type_map: BTreeMap<Type, i32>,
env: TypeEnv,
args: Vec<Type>,
result: Vec<u8>,
Expand All @@ -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(),
Expand All @@ -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);
Expand Down
24 changes: 12 additions & 12 deletions rust/candid/src/types/internal.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -33,8 +33,8 @@ pub fn type_of<T>(_: &T) -> TypeId {

#[derive(Default)]
struct TypeName {
type_name: HashMap<TypeId, String>,
name_index: HashMap<String, usize>,
type_name: BTreeMap<TypeId, String>,
name_index: BTreeMap<String, usize>,
}
impl TypeName {
fn get(&mut self, id: &TypeId) -> String {
Expand Down Expand Up @@ -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<TypeInner>);

#[derive(Debug, PartialEq, Hash, Eq, Clone)]
#[derive(Debug, PartialEq, Hash, Eq, Clone, PartialOrd, Ord)]
pub enum TypeInner {
Null,
Bool,
Expand Down Expand Up @@ -382,7 +382,7 @@ pub fn text_size(t: &Type, limit: i32) -> Result<i32, ()> {
}
}

#[derive(Debug, Eq, Clone)]
#[derive(Debug, Eq, Clone, PartialOrd, Ord)]
pub enum Label {
Id(u32),
Named(String),
Expand Down Expand Up @@ -423,7 +423,7 @@ impl std::hash::Hash for Label {

pub type SharedLabel = std::rc::Rc<Label>;

#[derive(Debug, PartialEq, Hash, Eq, Clone)]
#[derive(Debug, PartialEq, Hash, Eq, Clone, PartialOrd, Ord)]
pub struct Field {
pub id: SharedLabel,
pub ty: Type,
Expand Down Expand Up @@ -486,13 +486,13 @@ macro_rules! variant {
}}
}

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub enum FuncMode {
Oneway,
Query,
CompositeQuery,
}
#[derive(Debug, PartialEq, Hash, Eq, Clone)]
#[derive(Debug, PartialEq, Hash, Eq, Clone, PartialOrd, Ord)]
pub struct Function {
pub modes: Vec<FuncMode>,
pub args: Vec<Type>,
Expand Down Expand Up @@ -626,9 +626,9 @@ pub fn unroll(t: &Type) -> Type {
}

thread_local! {
static ENV: RefCell<HashMap<TypeId, Type>> = RefCell::new(HashMap::new());
static ENV: RefCell<BTreeMap<TypeId, Type>> = const { RefCell::new(BTreeMap::new()) };
// only used for TypeContainer
static ID: RefCell<HashMap<Type, TypeId>> = RefCell::new(HashMap::new());
static ID: RefCell<BTreeMap<Type, TypeId>> = const { RefCell::new(BTreeMap::new()) };
static NAME: RefCell<TypeName> = RefCell::new(TypeName::default());
}

Expand Down
5 changes: 3 additions & 2 deletions rust/candid/tests/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,9 @@ fn test_struct() {
);

let list: Option<List> = None;
// without memoization on the unrolled type, type table will have 3 entries.
all_check(list, "4449444c026e016c02a0d2aca8047c90eddae70400010000");
// with memoization on the unrolled type, type table will have 2 entries.
// all_check(list, "4449444c026e016c02a0d2aca8047c90eddae70400010000");
all_check(list, "4449444c036e016c02a0d2aca8047c90eddae704026e01010000");
}

#[test]
Expand Down

0 comments on commit ae4d0f7

Please sign in to comment.