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

make SizeLimit a trait #124

Merged
merged 2 commits into from Mar 18, 2017
Merged

make SizeLimit a trait #124

merged 2 commits into from Mar 18, 2017

Conversation

@TyOverby
Copy link
Collaborator

TyOverby commented Feb 25, 2017

Closes #121

@TyOverby
Copy link
Collaborator Author

TyOverby commented Feb 25, 2017

@dtolnay
Copy link
Collaborator

dtolnay commented Feb 25, 2017

This didn't make much of a difference in #123...

Before

test bincode_deserialize ... bench:          42 ns/iter (+/- 0)
test bincode_serialize   ... bench:          82 ns/iter (+/- 1)

After

test bincode_deserialize ... bench:          36 ns/iter (+/- 1)
test bincode_serialize   ... bench:          84 ns/iter (+/- 4)

I don't have time to review the code tonight but maybe read through the code again and focus on places where the Infinite / &[u8] case has different behavior compared to my implementation.

src/lib.rs Outdated
Infinite,
Bounded(u64)
pub trait SizeLimit {
#[inline(always)]

This comment has been minimized.

@dtolnay

dtolnay Feb 25, 2017

Collaborator

My understanding is this doesn't do anything, the compiler just ignores it. The inline attribute needs to go where the body of the method is defined.

This comment has been minimized.

@TyOverby

TyOverby Feb 25, 2017

Author Collaborator

Oh dang.

@TyOverby
Copy link
Collaborator Author

TyOverby commented Feb 25, 2017

the serde-rs/bench test is a really micro-benchmark.

I think that I'm going to spend some time and make a benchmarking system that has a larger variety of tests.

This was referenced Feb 26, 2017
@TyOverby TyOverby force-pushed the size-limit-trait branch from c413e8d to 1a81a01 Mar 18, 2017
@TyOverby TyOverby merged commit d8eac92 into master Mar 18, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
bors-servo added a commit to servo/ipc-channel that referenced this pull request Mar 21, 2017
Switch to bincode 1.0.0-alpha6 Infinite struct

Ref: servo/bincode#124

`SizeLimit` has been refactored to a trait, and `Infinite` is now a struct which implements that trait.
@TyOverby TyOverby deleted the size-limit-trait branch Oct 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.