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

Don't pass blocks around, pass CIDs instead #86

Closed
daviddias opened this issue Dec 21, 2016 · 4 comments
Closed

Don't pass blocks around, pass CIDs instead #86

daviddias opened this issue Dec 21, 2016 · 4 comments
Labels
status/deferred Conscious decision to pause or backlog

Comments

@daviddias
Copy link
Member

As noted in: #76

The decision engine buffers the blocks in memory when it informs the message queue that there is one more block to send, this means that if thousands of transfers are being performed, every block will stay in memory till the message gets sent. A better approach would be to only read from disk when the message is going to be sent.

With this, we might loose some perf by having to go read from disk, but that is why I created this issue ipfs/js-ipfs-repo#110 to add a LRU cache on the repo.

@dignifiedquire
Copy link
Member

With the perf PR reads from the store are much more efficient amd will only happen when preparing for message sending. I don't believe that the suggested approach here would help regarding performance in any way.

@daviddias
Copy link
Member Author

With the perf PR reads from the store

Ok, this might be new, because it wasn't like that before perf PR, the full blocks were being passed and put in the "MessageQueue" of each peer, waiting for async to pick it up.

I don't believe that the suggested approach here would help regarding performance in any way.

You don't think that putting a LRU cache for reads in IPFS-Repo can help perf? Could you elaborate? Think about all disk reads or even worse, when the store is actually behind a network (i.e. S3)

@dignifiedquire
Copy link
Member

I don't know about the LRU cache we would need to benchmark this. I meant changing the way we pass blocks vs cids around at the moment.

@dignifiedquire
Copy link
Member

I might be missing something, would be great if you could point to some code in the latest version of the PR where you think this would help.

@daviddias daviddias added the status/deferred Conscious decision to pause or backlog label Dec 22, 2016
@momack2 momack2 added this to Backlog in ipfs/js-ipfs May 10, 2019
@momack2 momack2 added this to Backlog in ipfs/js-waffle May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/deferred Conscious decision to pause or backlog
Projects
No open projects
Development

No branches or pull requests

2 participants