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.

Sign up for GitHub

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

Jump to bottom

Performance regression in decoding struct type #240

Closed
chenyan-dfinity opened this issue Jun 4, 2021 · 2 comments
Closed

Performance regression in decoding struct type #240

chenyan-dfinity opened this issue Jun 4, 2021 · 2 comments
Labels
Rust

Comments

@chenyan-dfinity
Copy link
Contributor

chenyan-dfinity commented Jun 4, 2021 •
edited
Loading

With the new decoding code, there is a const factor overhead in decoding struct type, likely due to Type clone.

Benchmark for 5a76023

Click to view benchmark
Test PR Benchmark Master Benchmark %
Blob/&str 263.3±59.50µs 268.1±66.71µs -1.79%
Blob/ByteBuf 225.1±65.30µs 229.4±67.28µs -1.87%
Blob/Bytes 129.3±27.79µs 125.3±28.09µs +3.19%
Blob/String 302.5±65.60µs 303.0±67.89µs -0.17%
Collections/vec (text, nat) 94.4±2.55ms 94.2±1.80ms +0.21%
Collections/vec int 44.6±0.06ms 41.9±0.07ms +6.44%
Collections/vec int64 32.7±0.08ms 26.2±0.09ms +24.81%
Collections/vec nat8 24.4±0.04ms 19.1±0.03ms +27.75%
option list/1024 3.2±0.01ms 1185.5±2.33µs +169.93%
profiles/1024 5.6±0.02ms 2.7±0.01ms +107.41%
variant list/1024 3.0±0.01ms 1354.9±2.70µs +121.42%

Originally posted by @github-actions[bot] in #211 (comment)
@matthewhammer
Copy link
Contributor

matthewhammer commented Jul 13, 2021 •
edited
Loading

FWIW, I have a usecase that creates "large" trees (each under 500k bytes) and I notice a 10x difference between 0.6 and 0.7 (where 0.6 is 10x faster than 0.7).

Faster decoding (via 0.6):

decode time vs response size

Slower decoding (via 0.7):

decode time vs response size (1)

@matthewhammer matthewhammer mentioned this issue Jul 13, 2021
Merged
ninegua pushed a commit to ninegua/candid that referenced this issue Apr 22, 2022
@hansl
fix: make arguments for Authenticator optional, and use proper typing…
29dec45 
… in Command (dfinity#240)

The typing was not compatible with any existing Identity. Now at least it's
compatible with all of them (including WebAuthnIdentity and other Delegation).

Also added a demo of a sample JavaScript application using a simple AuthClient
built on top of the Authenticator class.
@chenyan-dfinity
Copy link
Contributor Author

Fixed in #405

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Rust
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@matthewhammer @chenyan-dfinity