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

More benchmarks #52

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from
Draft

More benchmarks #52

wants to merge 7 commits into from

Conversation

NULLx76
Copy link
Member

@NULLx76 NULLx76 commented Jun 15, 2019

No description provided.

@codecov
Copy link

codecov bot commented Jun 15, 2019

Codecov Report

Merging #52 into develop will increase coverage by 1.01%.
The diff coverage is 75%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #52      +/-   ##
===========================================
+ Coverage    61.73%   62.75%   +1.01%     
===========================================
  Files           23       23              
  Lines         1725     1893     +168     
  Branches       468      507      +39     
===========================================
+ Hits          1065     1188     +123     
- Misses         224      226       +2     
- Partials       436      479      +43
Impacted Files Coverage Δ
src/payloads/binmemberauthenticationpayload.rs 53.84% <ø> (ø) ⬆️
src/crypto/mod.rs 57.89% <0%> (+1.57%) ⬆️
src/community/peer.rs 81.08% <100%> (-1.68%) ⬇️
src/serialization/mod.rs 67.65% <100%> (-2.62%) ⬇️
src/community/mod.rs 70.83% <70.31%> (+18.2%) ⬆️
src/crypto/signature.rs 60.5% <71.42%> (-3.01%) ⬇️
src/networking/mod.rs 71.42% <77.77%> (+0.15%) ⬆️
src/crypto/keytypes.rs 57.08% <82.35%> (+1.94%) ⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf03586...b7bac93. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 15, 2019

Codecov Report

Merging #52 into develop will not change coverage.
The diff coverage is 80%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop      #52   +/-   ##
========================================
  Coverage    62.75%   62.75%           
========================================
  Files           23       23           
  Lines         1893     1893           
  Branches       507      507           
========================================
  Hits          1188     1188           
  Misses         226      226           
  Partials       479      479
Impacted Files Coverage Δ
src/crypto/mod.rs 57.89% <ø> (ø) ⬆️
src/serialization/mod.rs 67.65% <100%> (ø) ⬆️
src/crypto/signature.rs 60.5% <66.66%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 050fdbd...5550011. Read the comment docs.

@jdonszelmann jdonszelmann self-requested a review June 16, 2019 08:58
@codeclimate
Copy link

codeclimate bot commented Jun 16, 2019

Code Climate has analyzed commit 5550011 and detected 0 issues on this pull request.

View more on Code Climate.

@NULLx76
Copy link
Member Author

NULLx76 commented Jun 16, 2019

I also want to benchmark our throughput when using a work stealing threadpool, we could look at how tokio benchmarks this and do something similar

@ichorid
Copy link

ichorid commented Jun 17, 2019

I wonder why don't you use tokio for reactor/socket/threadpool? Is tokio too much for this? Or you wanted to minimize external dependencies?

@NULLx76
Copy link
Member Author

NULLx76 commented Jun 17, 2019

We did indeed try tokio but it was too much just like you said. The framework would "envelop" the application becoming comparable to twisted (requiring anyone that import the library to start it for example). We do use parts of tokio like mio which is basically the network backend of tokio, but we opted not to use the entire framework. The fact that it adds quite some external dependencies also factored in to this decision. We may reconsider using tokio when rust implements its own async which will happen in the not too distant future and also lessen the dependencies of tokio considerably, or if/when we see another reason (like performance) to do so.

We'll likely run some benchmarks to compare performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants