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

RFC: change default max channel size to typemax(Int) #17698

Closed
wants to merge 1 commit into from

Conversation

amitmurthy
Copy link
Contributor

Currently, if the size is unspecified, Channel() creates a channel with a max size of 32. The rationale for this at that time were

  1. Partially mimic the produce-consume lockstep task switching wherein each depended on the other. Having an unlimited max size would mean a producer task could just fill up a channel before any consumers were run.

  2. It was mainly relevant when the implementation used a circular buffer which grew dynamically but was never shrunk. Now the backing store is an Array which resizes depending on the number of elements present.

I think the default should either be 0, 1 or unlimited(i.e., typemax(Int)) with my preference for unlimited.

A max size of 0 (which is currently not supported) would mimic golang's channels which require produce and consume to run in tandem, i.e., a put! on a channel of size 0 would block till a take! is called.

@ViralBShah
Copy link
Member

What is the downside for unlimited?

@amitmurthy
Copy link
Contributor Author

Point 1 above - Having an unlimited max size would mean a producer task could fill up a channel before any consumers were run - especially since we don't have tasks switched on multiple threads yet. User code would need to explicitly specify an appropriate buffer size for the problem at hand. This is true only if the producer is purely compute bound and does not yield.

@kshyatt kshyatt added parallelism Parallel or distributed computation io Involving the I/O subsystem: libuv, read, write, etc. labels Jul 29, 2016
@tkelman
Copy link
Contributor

tkelman commented Jul 30, 2016

This is a behavior change and should wait until we branch IMO.

@StefanKarpinski StefanKarpinski added this to the 0.6.0 milestone Aug 2, 2016
@StefanKarpinski
Copy link
Member

My impression is that channels with unlimited buffers are a bit dangerous, no?

@amitmurthy
Copy link
Contributor Author

Pros and cons of various defaults:

0 - Producers cannot really start producing till consumers are active. Concurrency is unoptimized in the sense that producer code must wait for consumers to be active, priming a work queue that is ready for consumers will not happen. This is safe though - no runaway production.

1 - Similar to 0. A work queue is primed with 1 entry

32 (current) - Balance between a ready work queue and possible runaway production. Disadvantage is that 32 is pretty arbitrary and I have seen issues where this choice of default has affected concurrency.

unlimited - A work queue is fully primed. Ideal where production is regulated by other mechanisms. Disadvantage is that the programmer has to be aware of runaway channel filling depending on the circumstances.

I am veering around to Golang's unbuffered channels as the default now, i.e., a default of size 0. Safest and when documented users are aware that the buffer size has to be explicitly specified depending on the nature of the workload.

@StefanKarpinski
Copy link
Member

What about no default? Then someone has to read what that argument is and what the different values mean and what the tradeoffs are (similar to the above pro/con list).

@amitmurthy
Copy link
Contributor Author

No default is fine too.

@kshyatt kshyatt added the needs decision A decision on this change is needed label Aug 3, 2016
@StefanKarpinski
Copy link
Member

Let's go with that for now and if it becomes apparent in the future that
one of the choices is much more commonly correct, we can make it a default,
although I'm skeptical that saving 1-3 characters of typing will be worth
the lack of explicitness.

On Wednesday, August 3, 2016, Amit Murthy notifications@github.com wrote:

No default is fine too.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#17698 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJX_BBto6w0TTLQvjOBX1nry8Vp7kpQks5qcELCgaJpZM4JYGH8
.

@amitmurthy
Copy link
Contributor Author

Superseded by #18832

@amitmurthy amitmurthy closed this Oct 7, 2016
@tkelman tkelman deleted the amitm/channelsizes branch October 8, 2016 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io Involving the I/O subsystem: libuv, read, write, etc. needs decision A decision on this change is needed parallelism Parallel or distributed computation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants