time: example of After includes a bad practice

[snadrus]

Does this issue reproduce with the latest release? Yes

What operating system? All

What did you do?

Used the time.After example in production, specifically:
case <-time.Now().After(5*time.Minute):

What did you expect to see?

A sensible scaling of the example as load increased.

What did you see instead?

https://medium.com/@oboturov/golang-time-after-is-not-garbage-collected-4cbc94740082
Using this in a switch clause fails to account for the lack of on-demand garbage collection (documented) which (at scale) becomes a problem consuming GB of memory and causing GC churn.

[cagedmantis]

/cc @rsc

[ianlancetaylor]

@snadrus What do you recommend instead?

The GC behavior is clearly documented, and the alternative involves not using time.After at all. But still seems useful to have an example for time.After.

[snadrus]

The risk is with "select" statements. Adding that to the documentation could help.

An example could have goroutine blocking: https://play.golang.org/p/0eUOnvITpcg

[cespare]

@snadrus your example would be better written with time.Sleep.

[snadrus]

Then perhaps it would be best to advise against this function since there are no safe uses except those covered better by other functions.

…

On Fri, Dec 4, 2020, 6:12 PM Caleb Spare ***@***.***> wrote: @snadrus <https://github.com/snadrus> your example would be better written with time.Sleep. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#43009 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOU4LS5EW6K3PDY3PNCRQDSTGJHVANCNFSM4UN5XLHA> .

[ianlancetaylor]

There are many safe uses of time.After. And most such uses will be in select statements.

It's true that if you have many calls to select statements that use time.After with a long duration, and if those select statements normally terminate quickly, then you will steadily accumulate memory dedicated to timers. And that is why the documentation already says "The underlying Timer is not recovered by the garbage collector until the timer fires. If efficiency is a concern, use NewTimer instead and call Timer.Stop if the timer is no longer needed."

But it's perfectly fine to use time.After if you don't run the select statement many times, or if the duration of the time.After is short, or if the code often times out via time.After rather than because data is sent on one of the other channels.

[snadrus]

That's quite a list of undocumented gotchas. If you're content with it, then I suppose we can close this bug.

[ianlancetaylor]

I only count a single gotcha, and it is documented.

[snadrus]

That single root cause is 3 "use" gotchas;

• Used too frequently
• Used with too long a delay vs use
• Used in places with "too few" timeout failures