Skip to content
This repository has been archived by the owner on Apr 8, 2019. It is now read-only.

[pool] Pools should only return pointer types #86

Open
jeromefroe opened this issue Aug 18, 2017 · 1 comment
Open

[pool] Pools should only return pointer types #86

jeromefroe opened this issue Aug 18, 2017 · 1 comment

Comments

@jeromefroe
Copy link
Contributor

Our object pool implementation, much like the pool in go's sync package, works with interfaces. This means that pooling objects that are not pointer types will require an extra allocation when they are put into the pool because the objects must be cast into interfaces, and only pointers can be cast into interfaces without an allocation. This issue is discussed more here: https://go-review.googlesource.com/c/24371.

As an example, each time a slice is put into an object pool, the three bytes which comprise the slice will be allocated on the heap (admittedly, since the underlying storage for the slice will already be on the heap, the cost of the extra three bytes will likely be minimal in comparison to the savings of reusing the data backing the slice). For other objects which are not pointer types, there may be no benefit to the pool at all since the cast to an interface will cause the object to be allocated on the heap each time. As a result, we should investigate whether we are using our pools incorrectly anywhere. A linter which checks for this issue may be beneficial here.

@jeromefroe jeromefroe changed the title [pool] Pool should usually only return pointer types [pool] Pools should usually only return pointer types Aug 18, 2017
@jeromefroe jeromefroe changed the title [pool] Pools should usually only return pointer types [pool] Pools should only return pointer types Aug 18, 2017
@jeromefroe
Copy link
Contributor Author

jeromefroe commented Aug 21, 2017

Did some benchmarks and there didn't seem to be a significant difference in execution time between using slices versus pointers to slices:

Benchmark Name|Iterations|Per-Iteration|Bytes Allocated per Operation|Allocations per Operation
----|----|----|----|----
BenchmarkPoolM3XPutSlice           |  5000000 | 282 ns/op | 32 B/op | 1 allocs/op
BenchmarkPoolM3XPutPointerToSlice  |  5000000 | 327 ns/op |  0 B/op | 0 allocs/op
BenchmarkPoolSyncPutSlice          | 10000000 | 184 ns/op | 32 B/op | 1 allocs/op
BenchmarkPoolSyncPutPointerToSlice | 10000000 | 177 ns/op |  0 B/op | 0 allocs/op

Admittedly, these benchmarks don't take into account the overhead of the extra three words needed to be GC'ed in the future, but that cost will likely be minimal in comparison to the savings from reusing the data backing the slice.

I also took a cut at a custom linter for spotting use of the pools in the pool package of m3x with types which are larger than one word in size: https://github.com/jeromefroe/m3lint.

Overall, I think the cost is relatively minor and not worth the effort to refactor right now. Will leave the issue open in case we want to investigate some more in the future.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant