6 ms·
This article isn't bad... but it misses several important points of Go. I also note the article is 9 months old. In the hope that my criticism will be taken as
by Jabbles 14y ago
This article isn't bad... but it misses several important points of Go. I also note the article is 9 months old. In the hope that my criticism will be taken as constructive, with apologies for not writing detailed explanations:
1. Goroutines are not threads
2. type inference allows you to elide types in var declarations:
var host = flag.String(...
3. Go's convention is to use camel case, not underscores.
4. Calling os.Exit all over the place is unusual imo - it may be better to panic().
5. fmt.Fprintf exists
os.Stderr.WriteString(fmt.Sprintf...
6. An explanation of why the standard log package isn't suitable would be nice, although I see the format used is slightly different.
7. Ignored errors when writing. Why do you wish to sync at every packet?
8. Massive race condition by re-using b. Line 62 overwrites b. b is then passed to c.logger (68) and c.binary_logger (70) for them to process asynchronously. c.logger is then passed another byte slice (72), which forces it to finish using b. c.binary_logger is not passed anything else, allowing it to delay until the next time something is sent on that channel, which would be after b is overwritten by the next packet. I think that simply moving line 58 to between 61,62 would fix this.
The author has not quite grokked the concept of "don't share memory". b is shared, and undefined behaviour results :(
- sirclueless 14y ago> I think that simply moving line 58 to between 61,62 would fix this. Doing that would erase the whole benefit of allocating a static array in the first place (you really don't want to be allocating memory for every packet that comes through). The funny thing is the author correctly handles this type of synchronization problem elsewhere, see lines 97-100. The right thing to do is to wait for acknowledgement from both loggers before reusing shared memory, but that sort of defeats his broader argument: if you are explicitly guarding every shared memory access with a barrier, then you might as well have been using any shared memory language besides Go. Buffered channels in both directions used to implement barriers are every bit as error-prone and complex as using a reader-writer lock explicitly.
- Jabbles 14y ago> Doing that would erase the whole benefit of allocating a static array in the first place (you really don't want to be allocating memory for every packet that comes through). The benefit is dubious. Performance is unlikely to be important in a single-connection logger. My advice is to lean on the garbage collector. Allocate away. Buffered channels are certainly more complex, but if you're never sharing memory, shouldn't hurt you.
- abecedarius 14y agoHow are goroutines not threads? Do you mean because it's possible for them to communicate without shared mutable state? Edit: Oh, apparently you all mean OS threads. So say so. (For example, in Haskell they're called threads without any implication that each one is an OS thread. Haskell's not unusual that way.)
- dlsspy 14y agogoroutines are not threads because thread means something and goroutine means something else.
- lbarrow 14y agoThread usually means an in-process concurrency construct controlled by calls to the operating system. A goroutine is managed by the Go runtime. It's an important distinction between using many (read: hundreds) of OS threads in a single process isn't performant, while using hundreds or even tens of thousands of goroutines is just fine.
- michaelfairley 14y agoGoroutines are green threads[1], and thus are threads, just like Ruby 1.8s threads are threads. Also, you can easily run 10s of thousands of kernel threads (provided you decrease the threads' stack size). 1: http://en.wikipedia.org/wiki/Green_threads http://en.wikipedia.org/wiki/Green_threads
- AYBABTME 14y agoGo schedules its own 'threads' (goroutines) to run on one or many (OS) threads. In this case, threads refer to not-a-process units of execution in an operating system. For whatever its worth, Wikipedia defines threads as: the smallest sequence of programmed instructions that can be managed independently by an operating system scheduler. I think that's also the commonly accepted definition: that threads are related to the OS scheduler.
- tptacek 14y ago
- lobster_johnson 14y ago> 1. Goroutines are not threads They are not kernel-level threads, but they are threads in every other meaningful sense; they have their own stack and execution pointer, and data operations in goroutines are not guaranteed to be atomic with respect to other goroutines. While the current official Go implementation never preemptively schedules goroutines except on I/O and on runtime.Gosched(), nothing in the spec precludes a different, more kernel-thread-like scheduling system.
- justinhj 14y agoUsing the term thread interchangeably with goroutine causes confusion, otherwise why bother having a different name for them in the first place?
- danieldk 14y agoBecause there already exists a perfectly fine name for them: green threads (as opposed to operating system threads), using M:N threading. http://en.wikipedia.org/wiki/Green_threads http://en.wikipedia.org/wiki/Green_threads http://en.wikipedia.org/wiki/Thread_(computing)#M:N_.28Hybrid_threading.29 http://en.wikipedia.org/wiki/Thread_(computing)#M:N_.28Hybri... They are just a subclass of threads.
- stephen 14y agoYour critique (which seems valid; I'm not a Go expert) does not exactly instill confidence in the Pragmatic Programmer (PP) brand. When I read material from them, I would have assumed I'd be seeing "idiomatic code from an expert"...not "hey, I'm a newbie hacking my way around in a new language, here's what I typed out in 2 days of playing with it". I mean, I understand they're big on the "learning a new language on a regular basis thing", but when I see code published like this, specifically from their brand, I go into it thinking "okay, this should be high quality, idiomatic code." ...guess I'll be more careful with that assumption. (EDIT: If this is just some personal blog, I guess I'd understand; I saw "magazine" in the URL and got the impression this was published as part of their magazine/PDF series.)
- jufo 14y agoYes, it is part of their magazine: http://pragprog.com/magazines/2012-06 http://pragprog.com/magazines/2012-06 (also available in PDF, epub and mobi from http://pragprog.com/magazines http://pragprog.com/magazines)