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

Rewrite Channel #3912

Closed
wants to merge 1 commit into from
Closed

Conversation

firejox
Copy link
Contributor

@firejox firejox commented Jan 17, 2017

for the further multi thread support
I think the design of channel should be changed.

@kostya
Copy link
Contributor

kostya commented Jan 17, 2017

is it fixed #3900 #3862?

@kostya
Copy link
Contributor

kostya commented Jan 17, 2017

how it performance in threadring benchmark https://github.com/kostya/crystal-benchmarks-game#threadring?

@spalladino
Copy link
Contributor

Thanks for the PR @firejox. However, part of the team is currently working heavily on multi threading support, so I'd advise against submitting large changes relative to the design of channels, at least until this stabilizes.

Also, besides multithreading in particular, please refrain from submitting PRs that involve large changes without opening an issue to discuss it first: otherwise you risk spending a lot of time working on an implementation for something that was not to be accepted for some particular reason.

@firejox
Copy link
Contributor Author

firejox commented Jan 18, 2017

@kostya Yes, it would fix those issues. Every select will not run more than one times coroutine switch. and here is the performance https://gist.github.com/firejox/f9fb7c4b6d44501d0b1070c123a1fab6
it is slower than origin but still faster than other language.

@firejox
Copy link
Contributor Author

firejox commented Jan 20, 2017

@spalladino Sorry for without opening issue first. Now I open a issue to discuss this.

@firejox firejox force-pushed the rewrite-channel branch 4 times, most recently from a074dfd to 24dad64 Compare January 26, 2017 16:27
- Fixes crystal-lang#3900 crystal-lang#3862
- Fully thread safe except coroutine switch
@firejox firejox changed the title [WIP] rewrite channel Rewrite Channel Jan 26, 2017
@firejox
Copy link
Contributor Author

firejox commented Jan 26, 2017

Now the rewrite is done. 😃

@asterite
Copy link
Member

Channel and all other concurrency-related stuff are only handled by the core team. And also they are working on it so these changes won't be compatible with them.

@asterite asterite closed this Sep 29, 2017
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.

None yet

4 participants