Help with Go channels and select talk

[Egon Kocjan]

Hello

I'm preparing a short talk about Go channels and select. More specifically, I want to show what not to do. I chose a bidirectional communication channel implementation, because it seems to be a common base for a lot of problems but hard to implement correctly without using any extra goroutines. All the code is here: https://github.com/egonk/chandemo

1_1.go: easy with en extra goroutine (takes 1.2s for million ints)

2_1.go: nice but completely wrong

2_2.go: better but still deadlocks

2_3.go: correct but ugly and slow (takes more than 2s for million ints)

2_4.go: correct and a bit faster but still ugly (1.8s for million ints)

So my question: is there a better way of doing it with just nested for and select and no goroutines? Basically, what would 2_5.go look like?

Thank you

Egon

[Robert Engels]

Channels are designed to be used with multiple go routines - if you’re not you are doing something wrong. 

On Dec 6, 2019, at 8:32 AM, Egon Kocjan <eko...@gmail.com> wrote:



--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/82830a5d-2bd8-4324-890e-9ae7f5f0fbaf%40googlegroups.com.

[Egon Kocjan]

There are goroutines in the examples of course, just a single goroutine per bidi channel seems hard. By contrast, I've worked with actor systems before and they are perfectly fine with a single fiber.

On Friday, December 6, 2019 at 3:38:20 PM UTC+1, Robert Engels wrote:

Channels are designed to be used with multiple go routines - if you’re not you are doing something wrong. 

On Dec 6, 2019, at 8:32 AM, Egon Kocjan <eko...@gmail.com> wrote:



Hello

I'm preparing a short talk about Go channels and select. More specifically, I want to show what not to do. I chose a bidirectional communication channel implementation, because it seems to be a common base for a lot of problems but hard to implement correctly without using any extra goroutines. All the code is here: https://github.com/egonk/chandemo

1_1.go: easy with en extra goroutine (takes 1.2s for million ints)

2_1.go: nice but completely wrong

2_2.go: better but still deadlocks

2_3.go: correct but ugly and slow (takes more than 2s for million ints)

2_4.go: correct and a bit faster but still ugly (1.8s for million ints)

So my question: is there a better way of doing it with just nested for and select and no goroutines? Basically, what would 2_5.go look like?

Thank you

Egon

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golan...@googlegroups.com.

[Robert Engels]

A channel is much closer to a pipe. There are producers and consumers and these are typically different threads of execution unless you have an event based (async) system - that is not Go. 

On Dec 6, 2019, at 9:30 AM, Egon Kocjan <eko...@gmail.com> wrote:



To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/bdc57eb0-b26f-4364-87fb-241b0807e8ae%40googlegroups.com.

[robert engels]

To clarify, with Go’s very lightweight threads it is “doing the multiplexing for you” - often only a single CPU is consumed if the producer and consumer work cannot be parallelized, otherwise you get this concurrency “for free”.

You are trying to manually perform the multiplexing - you need async structures to do this well - Go doesn’t really support async by design - and it’s a much simpler programming model as a result.

[Egon Kocjan]

Agreed, I see goroutines in general as a big win. But what I intend to talk about in the presentation:

- we have two unidirectional flows of data resembling something like a TCP socket, easy to do with two goroutines with a for loop

- let's add caching, so some requests do not go to the server

- it would be tempting to just combine two goroutines into one and handle caching in a single loop without using locks (I see developers avoid atomics and locks if they don't have a lot of previous experience with traditional MT primitives)

- this is surprisingly difficult to do properly with Go channels, see my attempts: https://github.com/egonk/chandemo/blob/master/2_3.go and https://github.com/egonk/chandemo/blob/master/2_4.go

- it is easy to do in actor systems, just move the code for both actors into a single actor!

The lesson here is that select is not a nice and safe compose statement even if it appears so at the first glance, do not be afraid to use locks.

Of course, if somebody comes up with a better implementation than 2_3.go and 2_4.go, I would be very happy to include it in the talk.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/bdc57eb0-b26f-4364-87fb-241b0807e8ae%40googlegroups.com.

[Jason E. Aten]

@Egon, 

I'm sure many here would jump in and assist, but you need to help us to help you by spelling out, specifically and exactly, the problem(s) you want solved. A few sentences on each challenge should suffice.

[robert engels]

I’m sorry but your design is not comprehendible by me, and I’ve done lots of TCP based services. 

i think you only need to emulate classic TCP processing - a reader thread (Go routine) on each side of the connection using range to read until closed. The connection is represented by 2 channels - one for each direction.

I think you might be encountering a deadlock because the producer on one end is not also reading the incoming - so either restructure, or use 2 more threads for the producers.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/75d69b4e-4fb7-4f62-8011-f21e2a4c294a%40googlegroups.com.

[Egon Kocjan]

I'll try to clarify as best as I can, thanks again to anyone looking at this.

The simple server implementation of "output <- input+1" is here and it is not "under our control" - it's what we have to work with: https://github.com/egonk/chandemo/blob/master/server.go

The test runner or client is here: https://github.com/egonk/chandemo/blob/master/demo.go (it just pushes in ints and gets server replies back through a connection layer)

The deadlocks in 2_1.go and 2_2.go are caused by the simplistic and wrong implementation of bidi-comm, which is what I'll be illustrating. I have three working solutions - 1_1.go, 2_3.go, 2_4.go. So the question is, can we remove the extra goroutine from 1_1.go and make the code nicer to read than 2_3.go and 2_4.go. The extra goroutine that I'd like to be removed is started here:

https://github.com/egonk/chandemo/blob/master/1_1.go#L14 (line 14)

What I mean by removed - no go statement, replaced presumably by some kind of for/select combination.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/75d69b4e-4fb7-4f62-8011-f21e2a4c294a%40googlegroups.com.

[Robert Engels]

Probably not. Go is designed for 1:1 and there is no reason to do it differently. You could probably try to write an async event driven layer (which it looks like you’ve tried) but why???

It’s like saying I’d really like my plane to float - you can do that -but most likely you want a boat instead of a plane. 

On Dec 7, 2019, at 2:38 AM, Egon Kocjan <eko...@gmail.com> wrote:



To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/8b87adcc-2249-402c-b34c-20df5013860a%40googlegroups.com.

[Egon Kocjan]

I'll cite myself:

"I'm preparing a short talk about Go channels and select. More specifically, I want to show what not to do."

and

"it would be tempting to just combine two goroutines into one and handle caching in a single loop without using locks (I see developers avoid atomics and locks if they don't have a lot of previous experience with traditional MT primitives)"

Before I say one can't do something in Go, I wanted to ask here to make sure I'm not missing something obvious. Basically, I intend to show how difficult lock-free programming can be so don't force it - just use goroutines and locks.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/8b87adcc-2249-402c-b34c-20df5013860a%40googlegroups.com.

[Robert Engels]

I understand what you are saying but I’ll still suggest that your premise/design is not correct. There are plenty of useful lock free structures in Go (see github.com/robaho/go-concurrency-test) but that is not what you are attempting here... you are using async processing - these are completely different things. Using async in Go is an anti-pattern IMO. 

On Dec 8, 2019, at 12:11 AM, Egon Kocjan <eko...@gmail.com> wrote:



To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/3b9bb722-d43f-4e70-8384-dc17cdec6090%40googlegroups.com.

[Egon Kocjan]

I meant lock-free as in "without explicit locks".

The original challenge still stands if someone has a better solution than me:

"The deadlocks in 2_1.go and 2_2.go are caused by the simplistic and wrong implementation of bidi-comm, which is what I'll be illustrating. I have three working solutions - 1_1.go, 2_3.go, 2_4.go. So the question is, can we remove the extra goroutine from 1_1.go and make the code nicer to read than 2_3.go and 2_4.go. The extra goroutine that I'd like to be removed is started here:

https://github.com/egonk/chandemo/blob/master/1_1.go#L14 (line 14)"

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/3b9bb722-d43f-4e70-8384-dc17cdec6090%40googlegroups.com.

[Robert Engels]

I’m sorry, but it’s very hard to understand when you start with solutions. I think maybe clearly restating the problem will allow more people to offer up ideas. To be honest at this point I’m not really certain what you’re trying to demonstrate or why. 

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/4a176af0-74bb-49b5-ae4d-d8714c7bc46d%40googlegroups.com.

[luka....@gmail.com]

You can do it without a goroutine as long as the channel has at least one buffer slot so execution passes to the select and uses the data.

[Bryan C. Mills]

I agree. It seems to me that the problem in example 2 is deep in the architecture of the program, not just a detail of the `select` statements.

The `connect` function essentially functions as a single-worker “worker pool”, storing the data in a goroutine (specifically, in the closure of the `srv` function). The need for the channels at all seems unmotivated, and so the use of the channels seems inappropriate — I suspect that that is why you aren't finding a satisfactory solution.

Stepping up a level: Egon, you say that you “want to show what not to do.”

That is pretty much the premise of my GopherCon 2018 talk, Rethinking Classical Concurrency Patterns (https://www.youtube.com/watch?v=5zXAHh5tJqQ).

I would suggest going back to a more concrete problem and re-examining it with the advice of that talk in mind.

(If you would like more detail on how to apply that advice, I'd be happy to take a look at concrete examples — but I agree with Robert that the code posted earlier is too abstract to elicit useful feedback.)

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/4a176af0-74bb-49b5-ae4d-d8714c7bc46d%40googlegroups.com.

[Egon Kocjan]

Interesting suggestion, but I added the implementation with buffer = 1 here and it still deadlocks: https://github.com/egonk/chandemo/blob/master/2_3.go#L5

It stopped deadlocking when buffer = 5, so I think it will be a nice lesson about hiding design problems with buffers. Did you have something else in mind?

I've also simplified the correct solution and it looks like this now: https://github.com/egonk/chandemo/blob/master/2_4.go (not that bad, but still somewhat tricky async approach)

[Egon Kocjan]

My demo is based on a real problem in our code. The issue here is that I cannot paste proprietary code and it's large and complex anyway. I distilled the problem to a bidi-communication coding exercise. The "go srv" in the real code is an external process with stdin and stdout with a simple line based protocol. Both pipes are then presented as go channels: one line = one channel message.

This is a bit off-topic now, but coincidentally we will have another talk (probably by my work colleague) that is related to one of your approaches from the talk:

// Glob finds all items with names matching pattern

// and sends them on the returned channel.

// It closes the channel when all items have been sent.

func Glob(pattern string) <-chan Item {

We had a bug where due to panic+recover the channel consumer stopped reading from the channel prematurely and the producer deadlocked the entire process. I will argue that for exposed public API, sql-like defer Close / Next / Scan is safer than raw channels.

[Bryan C. Mills]

On Thu, Dec 12, 2019 at 1:17 AM Egon Kocjan <eko...@gmail.com> wrote:

My demo is based on a real problem in our code. The issue here is that I cannot paste proprietary code and it's large and complex anyway. I distilled the problem to a bidi-communication coding exercise. The "go srv" in the real code is an external process with stdin and stdout with a simple line based protocol. Both pipes are then presented as go channels: one line = one channel message.

Part of my point (and, I think, Robert's point) is that presenting the pipes as channels in the first place is the root of the problem.

If you avoid that mistake, then the concurrency problems down the stack become moot.

Or, equivalently: if the concrete problem is multiplexing input from two io.Readers, then start the presentation at the Readers — not the channels to which they have been converted. (The channels should be an internal implementation detail, not part of the higher-level API).

This is a bit off-topic now, but coincidentally we will have another talk (probably by my work colleague) that is related to one of your approaches from the talk:

// Glob finds all items with names matching pattern

// and sends them on the returned channel.

// It closes the channel when all items have been sent.

func Glob(pattern string) <-chan Item { 

We had a bug where due to panic+recover the channel consumer stopped reading from the channel prematurely and the producer deadlocked the entire process. I will argue that for exposed public API, sql-like defer Close / Next / Scan is safer than raw channels.

It seems that you didn't read/watch the whole talk, or even the whole section of the talk?

(I made exactly that point on slides 24–36. The asynchronous examples are the pattern to be rethought, not the final result!)

You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/1LZzZ0lE6ZA/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/31041bab-56be-4b91-978f-c22fb8f5a19f%40googlegroups.com.

[Egon Kocjan]

On Thursday, December 12, 2019 at 3:53:07 PM UTC+1, Bryan C. Mills wrote:

This is a bit off-topic now, but coincidentally we will have another talk (probably by my work colleague) that is related to one of your approaches from the talk:

// Glob finds all items with names matching pattern

// and sends them on the returned channel.

// It closes the channel when all items have been sent.

func Glob(pattern string) <-chan Item { 

We had a bug where due to panic+recover the channel consumer stopped reading from the channel prematurely and the producer deadlocked the entire process. I will argue that for exposed public API, sql-like defer Close / Next / Scan is safer than raw channels.

It seems that you didn't read/watch the whole talk, or even the whole section of the talk?

(I made exactly that point on slides 24–36. The asynchronous examples are the pattern to be rethought, not the final result!

Yes I'm sorry, I skimmed through the slides quickly. This is the actual library: https://github.com/emersion/go-imap