Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

Bounded #2164

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Bounded #2164

wants to merge 8 commits into from

Conversation

freesig
Copy link
Contributor

@freesig freesig commented Mar 19, 2020

PR summary

Adds bounded channels to prevent memory building up in unbounded channels

testing/benchmarking notes

( if any manual testing or benchmarking was/should be done, add notes and/or screenshots here )

followups

( any new tickets/concerns that were discovered or created during this work but aren't in scope for review here )

changelog

  • if this is a code change that effects some consumer (e.g. zome developers) of holochain core, then it has been added to our between-release changelog with the format
- summary of change [PR#1234](https://github.com/holochain/holochain-rust/pull/1234)

documentation

@@ -626,46 +628,51 @@ async fn handle_message_join_space(
) {
sim2h_handle
.state()
.clone()
Copy link
Contributor

@neonphog neonphog Mar 19, 2020

Choose a reason for hiding this comment

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

uhhh - if we clone the state here the new connection will get added to the clone and not the actual state - then get dropped.

Copy link
Contributor

Choose a reason for hiding this comment

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

scratch that - I see this is cloning the handle not the state : )

@neonphog
Copy link
Contributor

The sim2h side looks awesome! nice work async-ifying everything!

@zippy
Copy link
Member

zippy commented Mar 19, 2020

When I test manually I see lots of problems with not being able to get a lock on state in the action loop. Also you can't ctrl-c out of the conductor any more.

@neonphog
Copy link
Contributor

Using spawn_blocking for the for the crossbeam_channel blocking sends actually makes this playground work - although I at least get three would-block thread panics - so maybe 1000 is more than it can keep up with?

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=579de08c026ae6005e1c0ffdb319be54

@freesig
Copy link
Contributor Author

freesig commented Mar 25, 2020

Oh yeh I haven't figured out how to get the shutdown to happen on this properly yet. Maybe test it if and then if it's helpful I can look at how to do that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants