Do not store non-pointer values in sync.Pool. #4089
Conversation
A sync.Pool is used to avoid unnecessary allocations and reduce the amount of work the garbage collector has to do. When passing a value that is not a pointer to a function that accepts an interface, the value needs to be placed on the heap, which means an additional allocation. Slices are a common thing to put in sync.Pools, and they’re structs with 3 fields (length, capacity, and a pointer to an array). In order to avoid the extra allocation, one should store a pointer to the slice instead.
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ A review job has been created and sent to the PullRequest network.
@martinmr you can click here to see the review status or cancel the code review job.
There was a problem hiding this comment.
The change is pretty straightforward but this code is assuming that 100% of the time that route will be populated and we will never attempt to reference a nil pointer.
That can be a source of a lot of future issues if you don't create some assertion whenever this struct is created and used (that it'll always be there).
I would lean towards:
- adding some defensive programming to check whether the field is populated before accessing it OR
- ensuring that there is always a non-nil pointer populated in the struct at all times.
Reviewed with ❤️ by PullRequest
There was a problem hiding this comment.
Added an extra check.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @pawanrawal)
A sync.Pool is used to avoid unnecessary allocations and reduce the
amount of work the garbage collector has to do. When passing a value
that is not a pointer to a function that accepts an interface, the value
needs to be placed on the heap, which means an additional allocation.
Slices are a common thing to put in sync.Pools, and they’re structs with
3 fields (length, capacity, and a pointer to an array). In order to
avoid the extra allocation, one should store a pointer to the slice
instead.
This change is