Skip to content

perf: Compare proper CanisterIds as u64 values#10664

Open
alin-at-dfinity wants to merge 7 commits into
masterfrom
alin/CanisterId-as_u64
Open

perf: Compare proper CanisterIds as u64 values#10664
alin-at-dfinity wants to merge 7 commits into
masterfrom
alin/CanisterId-as_u64

Conversation

@alin-at-dfinity

@alin-at-dfinity alin-at-dfinity commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

If a CanisterId represents an actual canister (the majority case; as opposed to a SubnetId masquerading as a canister), it is actually a big-endian u64 value plus a couple of marker bytes underneath. Comparing such CanisterIds (both for equality but particularly for ordering) as u64 values gives speed-ups between 1.25x and 2.8x (with the less common path only slowing down by 6-20%). And because the u64 is encoded as big-endian, relative ordering remains the same.

While the CanisterId struct will grow by 1 byte (from 30 to 31 bytes) (i) that's all the increase (e.g. no extra 7 bytes due to memory alignment) and (ii) my guess is that in the vast majority of cases it actually takes up 32 bytes anyway (due to external alignment constraints).

We relatively rarely touch (or look up) the big BTreeMap of cold canisters anymore, but this will still be visible as faster message routing (and various canister call validations relying on the routing table). And faster HashTree creation.

Benchmark master this branch speedup
cmp u64 3.49 ns 1.71 ns 2.04× faster
binary_search 1024 80.5 ns 28.4 ns 2.83× faster
sort 1024 5.84 µs 3.79 µs 1.54× faster
eq u64, differ 2.22 ns 1.77 ns 1.25× faster
eq u64, equal 2.22 ns 1.76 ns 1.26× faster
linear_find (eq scan) 810 ns 638 ns 1.27× faster
cmp long/opaque 3.46 ns 3.68 ns ~6% slower
eq long, differ 2.46 ns 2.87 ns ~17% slower
eq long, equal 2.26 ns 2.70 ns ~20% slower
hash one u64 ID 32.4 ns 14.1 ns 2.30× faster
HashMap lookup, hit 35.9 ns 17.2 ns 2.10× faster
HashMap lookup, miss 31.0 ns 16.3 ns 1.90× faster
hash one long/opaque ID 31.2 ns 27.5 ns ~wash (both hash the full principal)

If a CanisterId actually represents a canister (the majority case; as opposed to a `SubnetId` masquerading as a canister), it is actually a `u64` value plus a couple of marker bytes underneath. Comparing such `CanisterIds` (both for equality but particularly for ordering) as `u64` values gives speed-ups between 1.25x and 2.8x (with the less common path only slowing down by 6-20%).

We relatively rarely touch (or look up) the big `BTreeMap` of cold canisters anymore, but this will result in faster routing (and various canister call validations relying on the routing table). And faster `HashTree` creation.

| Benchmark | `master` | this branch | speedup |
|---|---:|---:|---|
| `cmp` u64 | 3.49 ns | 1.71 ns | **2.04× faster** |
| `binary_search` 1024 | 80.5 ns | 28.4 ns | **2.83× faster** |
| `sort` 1024 | 5.84 µs | 3.79 µs | **1.54× faster** |
| `eq` u64, differ | 2.22 ns | 1.77 ns | **1.25× faster** |
| `eq` u64, equal | 2.22 ns | 1.76 ns | **1.26× faster** |
| `linear_find` (eq scan) | 810 ns | 638 ns | **1.27× faster** |
| `cmp` long/opaque | 3.46 ns | 3.68 ns | ~6% slower |
| `eq` long, differ | 2.46 ns | 2.87 ns | ~17% slower |
| `eq` long, equal | 2.26 ns | 2.70 ns | ~20% slower |
@alin-at-dfinity alin-at-dfinity requested review from a team as code owners July 6, 2026 12:41
@github-actions github-actions Bot added the perf label Jul 6, 2026
github-actions[bot]

This comment was marked as resolved.

@zeropath-ai

zeropath-ai Bot commented Jul 6, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 622ddfa.

Security Overview
Detected Code Changes
Change Type Relevant files
Refactor ► rs/registry/routing_table/src/lib.rs
    Replace manual canister_id to u64 conversion with canister_id.as_u64() and panic on invalid input
Enhancement ► rs/types/base_types/src/canister_id.rs
    Introduce CanisterId struct with is_u64 flag, updated equality, ordering, hashing, serialization, and conversions
► rs/types/base_types/src/principal_id.rs
    Add accessor helpers as_slice (const), len (const), and as_fixed_bytes (const)
Enhancement ► rs/types/base_types/BUILD.bazel
    Add bench target and bench configuration for canister_id
Enhancement ► rs/types/base_types/Cargo.toml
    Add criterion to dev-dependencies and enable bench harness
Enhancement ► rs/types/base_types/benches/canister_id.rs
    Add microbenchmarks for CanisterId::eq, CanisterId::cmp, collections, and hashing
Enhancement ► Cargo.lock, Cargo.Bazel.json.lock, Cargo.Bazel.toml.lock
    Update ic_principal and related crates to newer versions (0.1.5) and associated checksums
Enhancement ► bazel/rust.MODULE.bazel
    Update ic_principal version spec to ^0.1.5

@zeropath-ai

zeropath-ai Bot commented Jul 6, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to ffbabb8.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► rs/types/base_types/src/canister_id.rs
    Refactor CanisterId structure and methods to support a dual representation (u64-based and PrincipalId-based) with new helper methods and trait implementations
► rs/types/base_types/src/principal_id.rs
    Add const generic accessors (as_slice, len, as_fixed_bytes)
Enhancement ► rs/registry/routing_table/src/lib.rs
    Simplify canister_id_into_u64 to use canister_id.as_u64() with panic on invalid input
Enhancement ► rs/types/base_types/BUILD.bazel
    Add rust_bench rule for canister_id benchmarking
Enhancement ► rs/types/base_types/Cargo.toml
    Add criterion as dev-dependency and bench target for canister_id
Enhancement ► rs/types/base_types/src/canister_id.rs
    Comprehensive refactor of CanisterId: new fields is_u64 and id; implement PartialEq, Ord, PartialOrd, Serialize, CandidType; adjust ic_00, get_ref, get, unchecked_from_principal, from principal constructors; add is_canister_id and as_u64 helpers; update AsRef, AsRef<[u8]>, Display, From/Into conversions, TryFrom handling, and Deserialize integration
Enhancement ► rs/types/base_types/src/canister_id.rs
    Modify conversions to use new internal id field and adjust serialization/deserialization paths
Enhancement ► rs/types/base_types/src/canister_id.rs
    Introduce is_canister_id and as_u64 const functions for internal validation and conversion
Enhancement ► rs/types/base_types/src/principal_id.rs
    Extend PrincipalId with as_slice const, len const, and as_fixed_bytes const methods

@alin-at-dfinity alin-at-dfinity dismissed github-actions[bot]’s stale review July 6, 2026 12:54

No changes in behavior, it simply drops the custom implementation of ic_registry_routing_table::canister_id_into_u64() with a call to newly added CanisterId::as_u64() (whose output is the identical).

…or proper, u64 CanisterIds), add exception for len() without is_empty(), drop explicit return statment.
@alin-at-dfinity alin-at-dfinity enabled auto-merge July 6, 2026 14:23
Comment on lines +38 to +42
if self.is_u64 && other.is_u64 {
as_u64(self.id).cmp(&as_u64(other.id))
} else {
self.id.cmp(&other.id)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the conditions that must hold when implementing both PartialOrd and PartialEq is a.eq(&b) <==> a.partial_cmp(&b) == Some(Equal), though it looks like if self.is_u64 != other.is_u64, couldn't we still have a.partial_cmp(&b) == Some(Equal) but a.eq(&b) == False?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we could. a.is_u64 is equivalent to a.as_slice().len() == 10 && a.as_slice()[8] == 1 && a.as_slice()[9] == 1.

So self.is_u64 != other.is_u64 means that one of them has length 10 and ends in ..., 1, 1] and one either has a different length or it ends in different bytes. Regardless, cmp() would directly compare the slices and they could not possibly be equal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants