I don't think we need to hold people's hands with low-level things.

I don't think we need to hold people's hands with low-level things.

But isn't that what vet already does? It barks at you for incorrect use of unsafe.Pointer/uintptr, for example. (Note that I don't agree with putting this check in vet as it's an optimization.)

I'm intrigued. This is almost certainly a bug, in that if you're going to the bother of using sync.Pool, you really care about allocations, in which case you really want to know that you're doing it wrong. I'm curious how many problems such a check would turn up. It has a few nice properties--approximately no false positives, results very likely to be of significant interest, etc.

If you're going to the bother of using sync.Pool, you certainly already have a benchmark to show that your use of sync.Pool made things better. Your benchmark and existing profiling tools will quickly teach you about slice-in-interface allocating.

Plus, isn't cmd/vet about bugs, and not performance opportunities?

Does vet already warn about any other performance optimizations?

Your benchmark and existing profiling tools will quickly teach you about slice-in-interface allocating.

Good point. I'm convinced.

FWIW, I manually checked a few code bases of mine and found one instance where I had this bug. I checked, and I had converted to sync.Pool a few things in a single commit and benchmarked the overall performance, so I didn't immediately realize the bug.

Plus, isn't cmd/vet about bugs, and not performance opportunities?

Right. vet only flags code that has a high chance of being wrong, as in producing wrong results or running risk of crashes, races and so on.

As explained, this is not a suitable check to add to vet.

@bradfitz do you know each and every allocation on the HTTP roundtrip path? If not, you can't say that there is no one that is easily fixable and should not be there in the first place (a perf bug).
FWIW the first usage of sync.Pool that I reviewed internally did cache slices by value. Author did very extensive benchmarking and concluded that sync.Pool does not give significant benefit.

I suggest to update the documentation to mention this "requirement" prominently.

@bradfitz do you know each and every allocation on the HTTP roundtrip path?

I did back during the cycle where I was working on that. I knew it because I had our tools which showed me all of them.

FWIW the first usage of sync.Pool that I reviewed internally did cache slices by value.

I've written and reviewed code with problems too. Profiling tools help.

@rasky, feel free to file a new documentation bug. This one is closed.