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

do resolve operations concurrently #2197

Merged
merged 1 commit into from
Jan 15, 2016
Merged

do resolve operations concurrently #2197

merged 1 commit into from
Jan 15, 2016

Conversation

whyrusleeping
Copy link
Member

Retrieve the ipns record and public key concurrently for a bit of speedup on name resolves.

License: MIT
Signed-off-by: Jeromy jeromyj@gmail.com

@@ -132,23 +133,41 @@ func (r *routingResolver) resolveOnce(ctx context.Context, name string) (path.Pa
// /ipns/<name>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the "name should be a multihash" comment be above the block on line 125?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, probably. I'll go ahead and change that here

@hackergrrl
Copy link
Contributor

LGTM.

Appveyor seems to be confused ("Specify a project or solution file. The directory does not contain a project or solution file."). Known issue?

@dignifiedquire
Copy link
Member

appveyor config is not merged yet: #2137

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
}()

for i := 0; i < 2; i++ {
err = <-resp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this much faster than a WaitGroup? (waitgroups are less error prone)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

waitgroups don't allow us to easily return errors. Otherwise its functionally identical to a waitgroup

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahhh indeed

@jbenet
Copy link
Member

jbenet commented Jan 15, 2016

LGTM other than waitgroup comment o/

jbenet added a commit that referenced this pull request Jan 15, 2016
do resolve operations concurrently
@jbenet jbenet merged commit 3d58888 into master Jan 15, 2016
@jbenet jbenet deleted the feat/faster-resolve branch January 15, 2016 17:52
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.

4 participants