thanks for the clear repro @kenny
Why does aws-api use put!
instead of >!
https://github.com/cognitect-labs/aws-api/blob/954bd3c0376d84a7defe5215b9f2dee3059e421f/src/cognitect/aws/client.clj#L90?
I think that was a refactoring oversight.
That wasn't all inside a go block before.
Does response-ch https://github.com/cognitect-labs/aws-api/blob/954bd3c0376d84a7defe5215b9f2dee3059e421f/src/cognitect/aws/client.clj#L82 get garbage collected if region, creds, or endpoint result an anomaly? It seems like a bunch of channels could end up getting built up due to https://github.com/cognitect-labs/aws-api/blob/954bd3c0376d84a7defe5215b9f2dee3059e421f/src/cognitect/aws/client.clj#L107-L115 running even when region/creds/endpoint fetch result in an anomaly.
channels are not special with regards to gc
Then I'm pretty sure this code could result in a large amount of unused channels.
in this case you can think of response-ch as being a promise, which will be gc'ed when there are no references to it
Isn't https://github.com/cognitect-labs/aws-api/blob/954bd3c0376d84a7defe5215b9f2dee3059e421f/src/cognitect/aws/client.clj#L109 going to hold a reference to it forever if any branch besides the :else
is hit?
no
go blocks are not gc roots like threads are
a go block is transformed in to a callback that is registered on a channel, so if there are no other references to a channel, a channel will get gc'ed even if a go block is waiting to put or take from it
Ah, makes sense! Thanks for that explanation. So in this case, the gc will know immediately there will be no references to response-ch since the :else branch was not run, marking the channel for gc?
whenever the gc happens to run, if it decides to collect more garbage, it might eventually notice the channel is garbage
So it would follow, why is this code written this way instead of with a transducer?
that question makes no sense
Passing a transducer to the response-ch chan
https://github.com/cognitect-labs/aws-api/blob/954bd3c0376d84a7defe5215b9f2dee3059e421f/src/cognitect/aws/client.clj#L82 that does the mapping written in the https://github.com/cognitect-labs/aws-api/blob/954bd3c0376d84a7defe5215b9f2dee3059e421f/src/cognitect/aws/client.clj#L107.
i.e., that second go block seems unnecessary
Like this
Or, perhaps even better, this.
Easier to read a patch...
This impl seems cleaner & more efficient.
Cleaner is subjective. For me the separation of "here's all the stuff we do to make a request" from "here's what we do with a response" is easier to reason about.
"more efficient" is less so, of course 🙂
Are you just thinking of the extra go block, or something else?
Fair. And yes, the extra go block.
Got it.
I'm going to leave it as/is for now and get out the change to the thread pool.
I appreciate the input, @kenny. Thanks.
Of course! This library is great. Thanks for taking the time for discussion & maintenance.
Time well invested. I learn things.