803 823 #904
803 823 #904
Conversation
Next commit will add options to override these two. This is the first step towards exporting functionality that allows manually processing raw fetch responses.
This slab allocates the records we will be creating, reducing the number of allocs (and thus gc pressure), while keeping alloc size the sameish (or reducing it a bit). This should address the main problem for 823, but we can still add caching in a bit. For #823.
…esponses This incorporates #803 with a few minor changes. Thank you to @dimitarvdimitrov for the excellent contribution. Closes #803.
| // DefaultDecompressor returns the default decompressor used by clients. | ||
| // If a pool is provided and implements PoolDecompressBytes, it will be | ||
| // used. | ||
| func DefaultDecompressor(pools ...Pool) Decompressor { |
There was a problem hiding this comment.
is the intention here that only one pool is passed (and the function takes a variadic because that pool is optional)? It could be just a language thing, but that wasn't immediately clear to me.
There was a problem hiding this comment.
The first pool that implements PoolDecompressBytes will be used; the intention here is to allow the end user to use the same pools here that they use in the config option WithPools -- so, they can be a bit lazy if they have multiple separate pools rather than figure out which pool they should use.
There was a problem hiding this comment.
ahaaa. Is it the first or the last? Currently the loop in decompressor.Decompress takes a slice for all implementations and keeps only the last one.
Maybe mention this behaviour in a brief comment here as this is a public function (or a validation function and not rely on humans/AI reading comments),
There was a problem hiding this comment.
Actually you're pointing out a bug -- the first pool that returns a slice should cause the each function to return early. I'm going to change each arg to func(Pool) bool, and if it returns true, stop iterating.
| } | ||
|
|
||
| /*TODO | ||
| type PoolRequestBuffer interface { |
There was a problem hiding this comment.
did you mean to implement this in this PR or only later on?
There was a problem hiding this comment.
This will be implemented in this PR soon :)
| return | ||
| } | ||
|
|
||
| // Reset the capacity of slices to max to ensure we zero things and |
There was a problem hiding this comment.
nitpick on the comment: this only resets the length of the slices, their capacity is still the same. Also the sentence in the comment isn't complete
| var userPooled bool | ||
| d.pools.each(func(p Pool) { | ||
| if pdecompressBytes, ok := p.(PoolDecompressBytes); ok { | ||
| s := pdecompressBytes.GetDecompressBytes(src, codecType) |
There was a problem hiding this comment.
this will take a slice from every pool which implements the interface but will only return one. Should we add some safeguard this can't happen? Maybe some validation that there's at most one pool of each interface?
There was a problem hiding this comment.
This is a bug, have a fix locally that I'll push after adding the last pool.
| // DefaultDecompressor returns the default decompressor used by clients. | ||
| // If a pool is provided and implements PoolDecompressBytes, it will be | ||
| // used. | ||
| func DefaultDecompressor(pools ...Pool) Decompressor { |
There was a problem hiding this comment.
ahaaa. Is it the first or the last? Currently the loop in decompressor.Decompress takes a slice for all implementations and keeps only the last one.
Maybe mention this behaviour in a brief comment here as this is a public function (or a validation function and not rely on humans/AI reading comments),
No description provided.