Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use btreemap for type_map and release #542

Merged
merged 6 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading