7 ms·

This touches tangentially on a very interesting idea apart from contexts, at least for me as I've been recently learning Go: > it’s an anti-pattern for librari

by j1elo 3y ago

This touches tangentially on a very interesting idea apart from contexts, at least for me as I've been recently learning Go: > it’s an anti-pattern for libraries to start their own goroutines. Best practices dictate that you should perform your work synchronously and let the caller decide if they want it to be asynchronous. This is something I had not discovered yet, probably because it's just a "common knowledge" thing that doesn't get explained in the Go tutorials, but regardless seems like a good idea in general. It also coincidentally threads well with something I read yesterday: The bane of my existence: Supporting both async and sync code in Rust [1]. While not being well versed at all in Rust, I constantly had precisely this same thought: why not make a synchronous library by default, then let the application choose whether it wants to use it as-is, or to put an async runtime on top of it? It only makes sense to me, and I would apply this best practice from Go if I was trying to make a Rust library. Especially given that in Rust there is no "official" standard async runtime, so I believe that authors ought to not assume which runtime end users should be forced to depend on. [1]: https://news.ycombinator.com/item?id=39061839 https://news.ycombinator.com/item?id=39061839

jeffbee 3y ago
I agree with not forcing an execution model on your callers, if you can avoid it. I also try to extend the rule to be as flexible as possible for callers. For example, in C++, I don't like to see a function that returns a container. I prefer a function that accepts an output iterator as a parameter.
deleted 3y ago
[deleted]
Nullabillity 3y ago
There are two different questions here: 1. Should the code be async-aware? That is, should it be able to yield to other tasks? 2. Should the code launch background tasks? 1 is a cross-cutting concern, in a cooperative multitasking environment, anything that is unable to yield is, well, preventing other tasks from running. In Go, 1 is always true, and implemented by the runtime itself. In Rust, this means using async I/O, like preferring tokio over std. 2 is what the advice is about. The Rust equivalent here would be tokio::spawn. Writing async-independent Rust code requires you to tackle 1, maybe by writing a trait for all the I/O actions that you require from your executor, which could return blocking futures if you select to run synchronously. That said, blocking also prevents you from using async for structured concurrency in the library implementation, which may or may not be a big deal for your use-case.
disintegrator 3y ago
I think it's totally fine for libraries to spin up goroutines internally. My interpretation is that the library's public interface should appear to be synchronous. As a contrived example: package main import ( "context" "example.com/spider" ) func main() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() _, _ = spider.Crawl(ctx, "https://en.wikipedia.org") } In this case `Crawl` is a blocking call and, under the hood, it may well spin up a pool of goroutines to crawl a site. It's also really nice that context is available to tie the lifetimes of the main program (goroutine) to child goroutines without coloring functions (like with async-await). I used to work with the Go pubsub client (https://pkg.go.dev/cloud.google.com/go/pubsub https://pkg.go.dev/cloud.google.com/go/pubsub) a lot and that has a whole bunch of scheduling and batching functionality handled in goroutines and on the outside you're calling `topic.Publish(ctx, &pubsub.Message{Data: []byte("payload")})`.
- xyzzy_plugh 3y ago
  If you're willing to plumb through all the knobs imaginable then this is a fine approach. But if spider.Crawl just ran unbounded or with a fixed bound it could trivially become a huge headache. There are many patterns in Go that are preferable to just starting a bunch of concurrent work. For example the pubsub client has options to disable batching and limit in flight connections.
  - disintegrator 3y ago
    Indeed, it's down to the library API to give you the right knobs to control for concurrency. It says more about the quality of the library if it mismanages goroutines than it does about whether or not libraries should use goroutines at all.
  - twic 3y ago
    > There are many patterns in Go that are preferable to just starting a bunch of concurrent work. I have not worked in Go for several years, but i remember that when i did, more experienced people told me that this was exactly what you were supposed to do in Go. That where in another language you might set up a queue and a threadpool and so on, in Go, you should just spawn a load of goroutines, and let the runtime sort it out. Is this no longer the canonical approach?
randomdata 3y ago
> This is something I had not discovered yet No doubt because the Go team disagrees. They have been abundantly clear that, from their point of view, goroutines should be used and even used as part of the public API when it makes sense. That said, it is still probably really good advice for newcomers who won't have a good understanding for the cases where it does make sense, and especially because goroutines are the shiny thing every newcomer wants to play with and will try to find uses that don't make sense just to use it. As a rule, you don't want to force the caller into things they might not want. In the vast majority of cases, a synchronous API is what you will want to give them as it is the most versatile. And, really, that's something that applies generally. For example, in the case of the common (T, error) pattern, you will want to ensure T is always useful even when there is an error. The caller may not care about the error condition. That's not you for, the library author, to decide. The fewer assumptions you can make about the caller the better.
- tsimionescu 3y ago
  > For example, in the case of the common (T, error) pattern, you will want to ensure T is always useful even when there is an error. The caller may not care about the error condition. This applies to maybe 0.1% of functions. The overwhelming majority of functions, in the Go stdlib as well as real projects, that return (T, error) return an empty meaningless value for the error cases.
  - randomdata 3y ago
    Not my experience. Some very early code did not recognize this, but since then pretty much everyone has come to agree that values should always be useful. If you are writing a function today, there is no reason to not observe this. In practice, that typically means returning the zero value. To which idioms suggest that zero values should be useful. Rob Pike's Go Proverb[1] even states: "Make the zero value useful." Most commonly when returning (T, error) that zero value is nil. In Go, nil is a useful value! If the caller wants to observe the error state, great. But it is needlessly limiting if you force it upon them. That is not for the library author to decide. [1] https://go-proverbs.github.io https://go-proverbs.github.io
acaloiar 3y ago
> it’s an anti-pattern for libraries to start their own goroutines. Best practices dictate that you should perform your work synchronously and let the caller decide if they want it to be asynchronous. This line reminded me that we all need to beware of the "anti-pattern" police. Some develpers use this term effectively by explaining precisely why something is an "anti-pattern". But more often it's used as a way to shut down conversation and any actual critical thinking. There's a lot of nuance behind what makes something an "anti-pattern", and simply declaring something an anti-pattern isn't enough. "This is an anti-pattern and this is why ..." is enough. But I still avoid using the term regardless. FYI I'm not saying the author is the anti-pattern police, but it does sound like they've found themselves on the police's radar.
- chemcube 3y ago
  Hey. Author here. Yeah, the Go style guide at Google (the setting for the dcode review) is quite prescriptive. I don't mind labeling things "anti-patterns". It doesn't mean there's a law against using said pattern (if you're in a healthy org). It just means you should be able to justify why you go against the guideline. The whole reason I was comfortable attempting that approach in the first place is because I had successfully used that pattern before and was able to provide a rationale. It just happens that this case differed enough that using this pattern would've been unwise. You're right that things shouldn't just be classed as an anti-pattern without explaining why. And that's enforced in most Go-heavy orgs at Google. If you're gonna say something is bad without providing an explanation and linking to the relevant sources, the comment is likely to be closed or argued. I didn't provide an explanation for why this is considered an anti-pattern in this post because that wasn't the topic. But it appears most persons here are interested in that, so maybe I'll write a post on that next week :)
throwaway894345 3y ago
>> it’s an anti-pattern for libraries to start their own goroutines The specific argument here is that by writing sync functions, the library is more abstract because the caller can decide whether to run the function sync or async. I agree with this, but there are lots of areas where we could issue guidance to make libraries more abstract. For example, instead of a library function which returns a pointer to allocated memory (e.g., `NewFoo() Foo`) we should write functions which take pointers to memory and the caller can figure out whether to allocate them on the stack or the heap (e.g., `NewFoo(out Foo)`). I'm not advocating for this as a general rule of thumb because writing that kind of code in Go would not be very ergonomic, but there's a lot of performance-sensitive code even in the standard library that is written that way. Another example would be 'inversion of control', wherein functions take interface parameters and callers decide what implementation to pass in.
divan 3y ago
It might also come from the common sense of using the CSP model (goroutines and messages, basically). Being asynchronous or synchronous is not the property of the function; it's the caller's decision. So every function is "synchronous" and it's up to the caller to decide when to spin it into the background and what for. As others commented, it doesn't preclude from you using goroutines in libraries if you _really need to_. It is just important to remember that it's not obvious to the caller. In light of this "common sense", async/await concurrency concept doesn't make any sense. Why would function dictate how exactly it should be called by a caller? Is "watching TV" an async or sync action? Depends on the caller – whether they put all their attention into this action or doing it "in the background" while performing other tasks. It's not an inherent property of the "whatching TV" function. I have no idea why so many people think that async/await is a good idea for expressing concurrent systems.
- NorthTheRock 3y ago
  I've always seen this as the exact opposite view - from go's concurrency model, every function is "synchronous" so the caller is not given a choice, if they want to run it asynchronously they have to create a new thread, then if they care about the result deal with inter-thread communication. With async/await, you're explicitly giving control to the caller to decide, you can await this promise now and have the thread treat it as synchronous, you can spawn a new task to run it in the background, or you can join it with other promises where you don't care about order and await for the results of the group.
  - divan 3y ago
    Interesting. I see two different aspects here: 1) Mental model. My claim comes from the firm belief that the more code is aligned with how we think, the easier it is to reason about the code. I naturally think about actions as they are not async or sync by nature – rather, it's me who's in charge of how the action is going to be executed (back to my "watching TV" example). Human attention here serves as an analogy to utilizing the logical CPU core during runtime. 2) Performance consideration. What you described indeed can work, too, but it comes at a cost. With Go, yes, you have to handle async results yourself (if you care about results), but you now understand the price of this and can make better judgments of the code and complexity and have better performance overall.

by j1elo 3y ago

jeffbee 3y ago

I agree with not forcing an execution model on your callers, if you can avoid it. I also try to extend the rule to be as flexible as possible for callers. For example, in C++, I don't like to see a function that returns a container. I prefer a function that accepts an output iterator as a parameter.

deleted 3y ago

[deleted]

Nullabillity 3y ago

There are two different questions here: 1. Should the code be async-aware? That is, should it be able to yield to other tasks? 2. Should the code launch background tasks? 1 is a cross-cutting concern, in a cooperative multitasking environment, anything that is unable to yield is, well, preventing other tasks from running. In Go, 1 is always true, and implemented by the runtime itself. In Rust, this means using async I/O, like preferring tokio over std. 2 is what the advice is about. The Rust equivalent here would be tokio::spawn. Writing async-independent Rust code requires you to tackle 1, maybe by writing a trait for all the I/O actions that you require from your executor, which could return blocking futures if you select to run synchronously. That said, blocking also prevents you from using async for structured concurrency in the library implementation, which may or may not be a big deal for your use-case.

disintegrator 3y ago

I think it's totally fine for libraries to spin up goroutines internally. My interpretation is that the library's public interface should appear to be synchronous. As a contrived example: package main import ( "context" "example.com/spider" ) func main() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() _, _ = spider.Crawl(ctx, "https://en.wikipedia.org") } In this case `Crawl` is a blocking call and, under the hood, it may well spin up a pool of goroutines to crawl a site. It's also really nice that context is available to tie the lifetimes of the main program (goroutine) to child goroutines without coloring functions (like with async-await). I used to work with the Go pubsub client (https://pkg.go.dev/cloud.google.com/go/pubsub https://pkg.go.dev/cloud.google.com/go/pubsub) a lot and that has a whole bunch of scheduling and batching functionality handled in goroutines and on the outside you're calling `topic.Publish(ctx, &pubsub.Message{Data: []byte("payload")})`.

xyzzy_plugh 3y ago
If you're willing to plumb through all the knobs imaginable then this is a fine approach. But if spider.Crawl just ran unbounded or with a fixed bound it could trivially become a huge headache. There are many patterns in Go that are preferable to just starting a bunch of concurrent work. For example the pubsub client has options to disable batching and limit in flight connections.
- disintegrator 3y ago
  Indeed, it's down to the library API to give you the right knobs to control for concurrency. It says more about the quality of the library if it mismanages goroutines than it does about whether or not libraries should use goroutines at all.
- twic 3y ago
  > There are many patterns in Go that are preferable to just starting a bunch of concurrent work. I have not worked in Go for several years, but i remember that when i did, more experienced people told me that this was exactly what you were supposed to do in Go. That where in another language you might set up a queue and a threadpool and so on, in Go, you should just spawn a load of goroutines, and let the runtime sort it out. Is this no longer the canonical approach?

randomdata 3y ago

> This is something I had not discovered yet No doubt because the Go team disagrees. They have been abundantly clear that, from their point of view, goroutines should be used and even used as part of the public API when it makes sense. That said, it is still probably really good advice for newcomers who won't have a good understanding for the cases where it does make sense, and especially because goroutines are the shiny thing every newcomer wants to play with and will try to find uses that don't make sense just to use it. As a rule, you don't want to force the caller into things they might not want. In the vast majority of cases, a synchronous API is what you will want to give them as it is the most versatile. And, really, that's something that applies generally. For example, in the case of the common (T, error) pattern, you will want to ensure T is always useful even when there is an error. The caller may not care about the error condition. That's not you for, the library author, to decide. The fewer assumptions you can make about the caller the better.

tsimionescu 3y ago
> For example, in the case of the common (T, error) pattern, you will want to ensure T is always useful even when there is an error. The caller may not care about the error condition. This applies to maybe 0.1% of functions. The overwhelming majority of functions, in the Go stdlib as well as real projects, that return (T, error) return an empty meaningless value for the error cases.
- randomdata 3y ago
  Not my experience. Some very early code did not recognize this, but since then pretty much everyone has come to agree that values should always be useful. If you are writing a function today, there is no reason to not observe this. In practice, that typically means returning the zero value. To which idioms suggest that zero values should be useful. Rob Pike's Go Proverb[1] even states: "Make the zero value useful." Most commonly when returning (T, error) that zero value is nil. In Go, nil is a useful value! If the caller wants to observe the error state, great. But it is needlessly limiting if you force it upon them. That is not for the library author to decide. [1] https://go-proverbs.github.io https://go-proverbs.github.io

acaloiar 3y ago

> it’s an anti-pattern for libraries to start their own goroutines. Best practices dictate that you should perform your work synchronously and let the caller decide if they want it to be asynchronous. This line reminded me that we all need to beware of the "anti-pattern" police. Some develpers use this term effectively by explaining precisely why something is an "anti-pattern". But more often it's used as a way to shut down conversation and any actual critical thinking. There's a lot of nuance behind what makes something an "anti-pattern", and simply declaring something an anti-pattern isn't enough. "This is an anti-pattern and this is why ..." is enough. But I still avoid using the term regardless. FYI I'm not saying the author is the anti-pattern police, but it does sound like they've found themselves on the police's radar.

chemcube 3y ago
Hey. Author here. Yeah, the Go style guide at Google (the setting for the dcode review) is quite prescriptive. I don't mind labeling things "anti-patterns". It doesn't mean there's a law against using said pattern (if you're in a healthy org). It just means you should be able to justify why you go against the guideline. The whole reason I was comfortable attempting that approach in the first place is because I had successfully used that pattern before and was able to provide a rationale. It just happens that this case differed enough that using this pattern would've been unwise. You're right that things shouldn't just be classed as an anti-pattern without explaining why. And that's enforced in most Go-heavy orgs at Google. If you're gonna say something is bad without providing an explanation and linking to the relevant sources, the comment is likely to be closed or argued. I didn't provide an explanation for why this is considered an anti-pattern in this post because that wasn't the topic. But it appears most persons here are interested in that, so maybe I'll write a post on that next week :)

throwaway894345 3y ago

>> it’s an anti-pattern for libraries to start their own goroutines The specific argument here is that by writing sync functions, the library is more abstract because the caller can decide whether to run the function sync or async. I agree with this, but there are lots of areas where we could issue guidance to make libraries more abstract. For example, instead of a library function which returns a pointer to allocated memory (e.g., `NewFoo() Foo`) we should write functions which take pointers to memory and the caller can figure out whether to allocate them on the stack or the heap (e.g., `NewFoo(out Foo)`). I'm not advocating for this as a general rule of thumb because writing that kind of code in Go would not be very ergonomic, but there's a lot of performance-sensitive code even in the standard library that is written that way. Another example would be 'inversion of control', wherein functions take interface parameters and callers decide what implementation to pass in.

divan 3y ago

It might also come from the common sense of using the CSP model (goroutines and messages, basically). Being asynchronous or synchronous is not the property of the function; it's the caller's decision. So every function is "synchronous" and it's up to the caller to decide when to spin it into the background and what for. As others commented, it doesn't preclude from you using goroutines in libraries if you _really need to_. It is just important to remember that it's not obvious to the caller. In light of this "common sense", async/await concurrency concept doesn't make any sense. Why would function dictate how exactly it should be called by a caller? Is "watching TV" an async or sync action? Depends on the caller – whether they put all their attention into this action or doing it "in the background" while performing other tasks. It's not an inherent property of the "whatching TV" function. I have no idea why so many people think that async/await is a good idea for expressing concurrent systems.

NorthTheRock 3y ago
I've always seen this as the exact opposite view - from go's concurrency model, every function is "synchronous" so the caller is not given a choice, if they want to run it asynchronously they have to create a new thread, then if they care about the result deal with inter-thread communication. With async/await, you're explicitly giving control to the caller to decide, you can await this promise now and have the thread treat it as synchronous, you can spawn a new task to run it in the background, or you can join it with other promises where you don't care about order and await for the results of the group.
- divan 3y ago
  Interesting. I see two different aspects here: 1) Mental model. My claim comes from the firm belief that the more code is aligned with how we think, the easier it is to reason about the code. I naturally think about actions as they are not async or sync by nature – rather, it's me who's in charge of how the action is going to be executed (back to my "watching TV" example). Human attention here serves as an analogy to utilizing the logical CPU core during runtime. 2) Performance consideration. What you described indeed can work, too, but it comes at a cost. With Go, yes, you have to handle async results yourself (if you care about results), but you now understand the price of this and can make better judgments of the code and complexity and have better performance overall.