Skip to content

ipfs pin ls <not-pinned-hash> takes very long and uses lots of CPU #3783

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

Closed
hsanjuan opened this issue Mar 14, 2017 · 6 comments
Closed

ipfs pin ls <not-pinned-hash> takes very long and uses lots of CPU #3783

hsanjuan opened this issue Mar 14, 2017 · 6 comments
Assignees

Comments

@hsanjuan
Copy link
Contributor

Version information: 0.4.7-rc1

Type: Bug

Priority: P3

Description:

While ipfs pin ls <pinned hash> works fast. ipfs pin ls <unpinned hash> takes forever to inform the user that the hash is not pinned. I also observe around 5 secs 200% CPU usage until the request comes back. Tested with 0.4.5 and happens at least since then.

@hsanjuan hsanjuan self-assigned this Mar 14, 2017
@hsanjuan
Copy link
Contributor Author

I'll dig this up

@Kubuxu
Copy link
Member

Kubuxu commented Mar 14, 2017

If you have 5s there, you can capture CPU profile and it will give you some data.

@whyrusleeping
Copy link
Member

@hsanjuan its probably enumerating the entire pinset and making sure that the hash youre giving it isnt anywhere in any of the indirect pins

@hsanjuan
Copy link
Contributor Author

@whyrusleeping ipfs pin ls takes shorter. Isn't that enumerating the whole pin set too? (btw, my pinset is rather small)

@whyrusleeping
Copy link
Member

@hsanjuan If youre listing on a recursive or direct pin, it will see that its in the immediate pinset and short circuit. When its not in the top level set, it has to check whether or not its referenced indirectly. You might try just checking if your pin is --type=recursive

@Kubuxu Kubuxu added the need/analysis Needs further analysis before proceeding label Mar 15, 2017
@hsanjuan
Copy link
Contributor Author

hsanjuan commented Mar 20, 2017

The problem is that when checking if a single pin exists, the code takes every recursive pin and does a hasChild(cid), where hasChild calls recursively on the links from every child.

This means that branches will be recursively explored over and over even when they are shared by different parents. On the other side ipfs pin ls is faster because when doing pinLsAll(), previously visited branches are pruned in EnumerateChildren() because the cid is already part of the visited set.

I think it would make sense have the same approach as enumerateChildren() and prune already visited branches when searching if a pin exists.

hsanjuan added a commit that referenced this issue Mar 21, 2017
This avoids revisiting already-searched branches and cut down
the IsPinned() check times considerably when recursive pins
share big underlying DAGs.

License: MIT
Signed-off-by: Hector Sanjuan <[email protected]>
@hsanjuan hsanjuan added status/in-progress In progress and removed need/analysis Needs further analysis before proceeding labels Mar 21, 2017
hsanjuan added a commit that referenced this issue Mar 22, 2017
This avoids revisiting already-searched branches and cut down
the IsPinned() check times considerably when recursive pins
share big underlying DAGs.

A test has been added which double-checks that pinned and unpinned items
lookups respond as expected with shared branches.

License: MIT
Signed-off-by: Hector Sanjuan <[email protected]>
hsanjuan added a commit that referenced this issue Mar 22, 2017
This avoids revisiting already-searched branches and cut down
the IsPinned() check times considerably when recursive pins
share big underlying DAGs.

A test has been added which double-checks that pinned and unpinned items
lookups respond as expected with shared branches.

License: MIT
Signed-off-by: Hector Sanjuan <[email protected]>
hsanjuan added a commit that referenced this issue Mar 22, 2017
This avoids revisiting already-searched branches and cut down
the IsPinned() check times considerably when recursive pins
share big underlying DAGs.

A test has been added which double-checks that pinned and unpinned items
lookups respond as expected with shared branches.

License: MIT
Signed-off-by: Hector Sanjuan <[email protected]>
hsanjuan added a commit that referenced this issue Mar 22, 2017
This avoids revisiting already-searched branches and cut down
the IsPinned() check times considerably when recursive pins
share big underlying DAGs.

A test has been added which double-checks that pinned and unpinned items
lookups respond as expected with shared branches.

License: MIT
Signed-off-by: Hector Sanjuan <[email protected]>
hsanjuan added a commit that referenced this issue Mar 22, 2017
This avoids revisiting already-searched branches and cut down
the IsPinned() check times considerably when recursive pins
share big underlying DAGs.

A test has been added which double-checks that pinned and unpinned items
lookups respond as expected with shared branches.

License: MIT
Signed-off-by: Hector Sanjuan <[email protected]>
whyrusleeping added a commit that referenced this issue Mar 23, 2017
Fix #3783: Improve IsPinned() lookups for indirect pins
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

3 participants