Usage of time.Sleep to control loops

[lorddoig]

Hey, I've only had a quick play around but this seems like a great library. This kind of pseudo-native application architecture is right up my street, especially in go!

I was wondering, though - is there a reason for loops regulating themselves with time.Sleep? I could be missing something as I only ran a handful of the bundled examples but all of them shot to an idle CPU usage of 70-100%. I changed dispatcher.RunLoop to

outChannels := connection.GetOutputChannels()
for {
    response := <-outChannels.CommandResponses
    Dispatch(response)
}

which caused the program to idle at a normal (tiny) rate and didn't seem to break anything.

In connection.Reader there is the loop

for {
    select {
    case quit := <-in.Quit
       // Quit
    default:
        // Stuff
    }
    time.Sleep(time.Microsecond * 100)
}

which could (should?) cause the same behaviour (but didn't). Is there a reason it's not written

for {
    select {
    case quit := <-in.Quit
       // Quit
    case <-time.After(time.Microsecond *100)
        // Stuff
    }
}

?

Also, isn't the sleep on line 140 of connection.Writer redundant?

I promise these are actual questions and not veiled criticisms. Most of this is on the side anyway, the actual issue is the CPU usage. On another note, are you looking for contributions? I like this codebase.

[miketheprogrammer]

I am relatively new to GoLang in general, and not completely aware of all the best practices. The example code you provided definitely loops more idiomatic. I suppose we could fix the idleness with a blocking channel and a goto statement. I would appreciate anyone with experience here to help out by providing either resources or a pull request.

I definitely have not written the best code out their, I hope that community collaboration around a strong idea can lead to everyone learning and contibuting to something good and new.

[lorddoig]

Sure I get that. I really didn't mean to criticize for the sake of it - I was just a little confused because the rest of the code I looked at didn't read like a newcomer's work (not that I feel entirely comfortable making that judgement...) so I assumed there must have been a reason I wasn't seeing.

[miketheprogrammer]

@lorddoig I did not take it as criticism, nor did i mean to close the ticket, it was late at night here and i was responding fast. If anyone i spent the last 30 minutes of my night laying in bed, contemplating the best way to remove the constant load. I think we can seperate out the go routines and make them each handle a single channel in a loop. This will allow them to successfully block. This should improve peformance in general.

[miketheprogrammer]

@lorddoig What do you think is the best way to access multiple event styles in a blocking manner. I would like to keep type safety if at all possible. Primarily i think we can split major things into its own goroutines i.e

        case response := <-in.CommandResponses:
            Log.Info("CommandResponse Marhshaling.")

        case command := <-in.Commands:
            ActionId += 1
            command.ID = ActionId

We can change that pretty easily to two seperate goroutines that are blocking, but what if we need to tell the goroutine to quit, I dont really use the quit messaging yet, pretty much these routines should last for the lifetime of the application

[lorddoig]

@miketheprogrammer I've just woken up, and am a little sleepy myself, but a little later, once I've woken up, I'll take a closer look at the goroutines and try to get a better feel for the code.

As regards the quit message, I believe the idiomatic way to do it is either a) to give each goroutine it's own own quit channel which it registers with some kind of 'quit dispatcher' who sends each one a quit message at the appropriate time; or b) to keep track of the number of goroutines listening on a shared quit channel and pump exactly that many quit messages in when the time comes.

[miketheprogrammer]

@lorddoig Ah, my newishness to Golang has bitten me. Select statements are blocking i believe as long as you dont use a default. If I remove all the default: cases then the threads have no need for sleep as they will block until one of the channels is ready

[lorddoig]

@miketheprogrammer Yes, I believe so, but I've been in Clojure mode for about a month so I'm hesitant to make declarations of truth until I've refreshed my memory!

I know the issues board isn't really designed for general chit-chat, but I'm currently trying to get the go plugin for IntelliJ to play nice with this project, which is always a challenge - what do you use?

[miketheprogrammer]

Sublime with GoSublime, its brilliant and uses gofmt, goimports, and several other technologies. The greatest aspect is that you can write your code in your indentation level but the filesystem will always be 8 indentation spaces. I particularly write my code with 2 space indents, but as you can see the entire source tree is 8 space indents.

[tehbilly]

I realize I'm a little late chiming in here, but I just found this project last night. I'd considered making an effort to help make the code more idiomatic, and haven't had a chance to go through all of the source yet.

Using select (without a default case) across channels is absolutely the best way to handle this particular issue.

In terms of a "quit" check, you can close a channel and check for that where you need to. Or you can have a shutdown/cleanup area that has a slice of channels to notify when they need to close. I prefer the second method myself.

I'll try to find areas and look over posted issues to see where I can contribute directly, but since this issue has gone all chatty anyways I thought I'd chime in! Do you want to set up a gitter.im room? I'd be happy to hop in and chat/help throughout the day. Easier to keep track of than issue comments for me.

[miketheprogrammer]

I will set up a gitter.im room as well and put in on the readme. However I an most of the thrust community are active in #breach on freenode.

I love contributions, and love to learn from seeing better code, or even just different code.

[miketheprogrammer]

Idle states have been fixed, still more improvements to come in wip-0.4.0