Skip to content

feat(bitswap): synchronous close - #700

Closed
btc wants to merge 1 commit into
masterfrom
feat/bitswap-synchronous-close
Closed

feat(bitswap): synchronous close#700
btc wants to merge 1 commit into
masterfrom
feat/bitswap-synchronous-close

Conversation

@btc

@btc btc commented Jan 30, 2015

Copy link
Copy Markdown
Contributor

Now, bitswap closes synchronously.

I'm not happy with the performance hit. Not happy with the added complexity either.

Added this functionality to try to stop bitswap from sending and receiving data on the network after the core (and the network) closes. This issue isn't fully resolved. After bitswap closes, messages can still enter from the network. That should be addressed in a future PR.

@btc btc added the status/in-progress In progress label Jan 30, 2015
@jbenet

jbenet commented Jan 31, 2015

Copy link
Copy Markdown
Member

I'm not happy with the performance hit.

here specifically it is the two selects on HasBlock and GetBlocks? or what perf hit is there?

Not happy with the added complexity either.

Yeah :/ it isnt great. possible mitigation?

@whyrusleeping

Copy link
Copy Markdown
Member

I think i may have missed a couple conversations, why does it feel like we are abandoning contexts?

@whyrusleeping

Copy link
Copy Markdown
Member

as far as perf, realistically, we wont call GetBlocks often enough to notice the hit, its not really in a critical path.

@btc

btc commented Feb 2, 2015

Copy link
Copy Markdown
Contributor Author

I think i may have missed a couple conversations, why does it feel like we are abandoning contexts?

This isn't an abandonment of contexts. It's a shift in responsibilities.

Processes are used for many purposes. The following focuses on processes through the lens of managing the life-cycles of large objects.

Sub-systems like bitswap and dht make use of private, async workers. To ensure workers terminate when we expect, systems must manage them.

Contexts send termination signals from parent to child, but without additional book-keeping, parents cannot know when its child is terminated. Processes are this additional book-keeping. They are WaitGroups in the large, designed to work without manual reference counting.

Contexts are useful for maintaining control over long-running function calls. Processes are useful for managing private asynchronous workers.

@jbenet jbenet modified the milestone: α Feb 2, 2015
@whyrusleeping

Copy link
Copy Markdown
Member

Ah, gotcha. This LGTM

@btc

btc commented Feb 2, 2015

Copy link
Copy Markdown
Contributor Author

merged in c114b04

@btc btc closed this Feb 2, 2015
@btc btc removed the status/in-progress In progress label Feb 2, 2015
@btc
btc deleted the feat/bitswap-synchronous-close branch February 2, 2015 09:16
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