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

Catch up summary 2018-11-21 #16

Closed
9 tasks done
alanshaw opened this issue Nov 22, 2018 · 9 comments
Closed
9 tasks done

Catch up summary 2018-11-21 #16

alanshaw opened this issue Nov 22, 2018 · 9 comments

Comments

@alanshaw
Copy link
Member

alanshaw commented Nov 22, 2018

Apologies I wanted to get this done yesterday. Hopefully this captures our conversation:

Summary

@Elexy gave a presentation demoing the current setup. 3 basic benchmark tests currently.

Infrastructure requirements were ironed out with @eefahy. See conversation here ipfs/infra#452 (comment) for details.

@mcollina identified a perf issue for node initialization and documented here ipfs/js-ipfs#1727

Agreed a weekly catch up at the same time (Wednesday 15:00).

Action items

@alanshaw
Copy link
Member Author

You can specify the bits for the generated key - 512 is really fast but you can elminate it completely using the privateKey param https://github.com/ipfs/js-ipfs#optionsinit

Note that you might want to use this module for spawning nodes https://github.com/ipfs/js-ipfsd-ctl which might come in useful when we start benchmarking against a Go IPFS daemon.

In the catch up we spoke about whether the node initialization time should be part of the benchmarking tests and we decided it should not. Can we add a simple initialization speed benchmark for js/go?

@mcollina
Copy link
Contributor

@alanshaw can you please verify if the info in https://github.com/ipfs/benchmarks#run-tests-locally is sufficient for you to run things locally?

@mcollina
Copy link
Contributor

You can specify the bits for the generated key - 512 is really fast but you can elminate it completely using the privateKey param https://github.com/ipfs/js-ipfs#optionsinit

Is a key of length 512 secure enough? I think the current recommendation is 2048 for RSA. If it is safe, why is not the default? What are the practical limitations of using a pre-generated key in real-world applications? I don't think a user could pre-generate a key, or that could be generated and downloaded from somewhere else, right?

@Elexy
Copy link
Contributor

Elexy commented Nov 22, 2018

just a thought; can't you generate the key as part of npm install ?

@alanshaw
Copy link
Member Author

Is a key of length 512 secure enough? I think the current recommendation is 2048 for RSA. If it is safe, why is not the default?

2048 is the default - 512 is not secure enough. In the future we need to restrict the bits parameter to discourage users from generating insecure keys. The Go implementation already does this with the minimum set at 1024.

What are the practical limitations of using a pre-generated key in real-world applications? I don't think a user could pre-generate a key, or that could be generated and downloaded from somewhere else, right?

I'm not 100% sure I understand the question. I suggested these methods as potential solutions for initialising nodes in the benchmarking tests to avoid a long initialization time. I'm not recommending them as best practice for building real-world apps.

just a thought; can't you generate the key as part of npm install ?

I like this idea! It could potentially work if a user is installing ipfs globally (npm i -g ipfs) but as a dependency we can't know the repo location (where the key should be stored) at this time.

@mcollina
Copy link
Contributor

I'm not 100% sure I understand the question. I suggested these methods as potential solutions for initialising nodes in the benchmarking tests to avoid a long initialization time. I'm not recommending them as best practice for building real-world apps.

Effective benchmarks reflect real-world usage. Removing the key generation unblocks us for discovering other issues, but the problem remains. I would like to avoid a case where we generate data that is not valuable because it does not reflect real-world usage.

@alanshaw
Copy link
Member Author

Effective benchmarks reflect real-world usage

Of course! As I understood it, the data for the current benchmarks is only being recorded after a node was initialized, started and ready. Do you think that if we passed a 2048 bit pregenerated key it will be close enough to real-world usage when the benchmarks begin? I think yes, it would be good enough for the time being, and we can make the switch when the real issue is addressed.

This is totally optional - if the initialization time isn't getting in your way then we can just leave it as it is for now and the overall time it takes to run the benchmarks will just get super fast when the real issue is resolved.

I didn't mean to imply that the problem should not be fixed! We do want to address it, the Node.js solution you identified is being tracked here libp2p/js-libp2p-crypto#130. If you'd like to work on that issue then that would be very awesome.

As I suggested above it would be great to have a benchmark for initialization so we can see this perf improvement when it happens!

@mcollina
Copy link
Contributor

mcollina commented Dec 4, 2018

I think all of what is described here on our side has been done.


Investigate potential issue running benchmarks with existing repo (@alanshaw)

is this ok @alanshaw?

Run locally and feedback (@alanshaw, @achingbrain, @hugomrdias)

Can you please test and verify?

@mcollina
Copy link
Contributor

Closing as it all happened.

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

No branches or pull requests

8 participants