Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

time: Tick docs don't mention CPU cost of "leaks" #17757

Closed
manishrjain opened this issue Nov 3, 2016 · 3 comments
Closed

time: Tick docs don't mention CPU cost of "leaks" #17757

manishrjain opened this issue Nov 3, 2016 · 3 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@manishrjain
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.7.3

What operating system and processor architecture are you using (go env)?

amd64, Arch Linux

What did you do?

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

Read the documentation here: https://golang.org/pkg/time/#Tick
And comprehend what it says, before reading any further. If you do, you would come out understanding the memory cost of this wrapper function. But, most likely, you won't come out understanding the CPU cost.

I ran into this problem, more details here:
https://forum.golangbridge.org/t/runtime-siftdowntimer-consuming-60-of-the-cpu/3773

A simple code snippet which would show the memory and CPU cost of this wrapper function. CPU cost is significantly more noticeable than memory cost: https://play.golang.org/p/vi9D0oZqOQ

What did you expect to see?

The documentation should clearly mention that the "leaks" could significantly affect your CPU usage, even more so than memory, because the "zombie" tickers (created by each iteration) are still doing work.

Such a notice would very actively discourage a nonchalant usage of this wrapper function. Memory leak is bad, but it's not nearly as noticeable in this case, as tickers actively hitting your CPU, doing an equivalent of a busy-wait.

Thought: Long term, IMO, this function should be discontinued, but that's just a thought, and it's not what I'm proposing here.

What did you see instead?

The convenience wrapper talks about "leaks," which typically engineers understand as memory leaks. However, memory leaks are insignificant compared to the CPU cost of running time.Tick in a fast iterating loop.

@josharian josharian changed the title time.Tick doc doesn't mention CPU cost of "leaks" time: Tick docs don't mention CPU cost of "leaks" Nov 3, 2016
@quentinmit quentinmit added Documentation NeedsFix The path to resolution is known, but the work has not been done. labels Nov 3, 2016
@quentinmit quentinmit added this to the Go1.8Maybe milestone Nov 3, 2016
@iand
Copy link
Contributor

iand commented Nov 4, 2016

I think it is self evident that any leak related to a timer function could conceivably include CPU costs.

Leak in general is typically understood to mean "resource leak" which could be any finite resource: memory, cpu time, file handles etc.

@manishrjain
Copy link
Author

manishrjain commented Nov 4, 2016

Hey @iand,

While you're technically right that "leak" could mean any resource, that's not the popular definition of leaks. Doing a Google search for [memory leak] shows 4.2M results. Doing a google search for [resource leak] and [cpu leak] show 370K and 660K results, respectively -- tiny in comparison to the former. Most developers would read these lines and come out thinking the doc is purely about memory leak. Anecdotal evidence:

https://twitter.com/KarlKFI/status/793643218336227329
https://twitter.com/vCabbage/status/793651308360179712

And memory leak makes sense. It's a common problem with C++ programs, and when GC can't reach something, memory leaking is already expected and quickly assumed. Typically, these leaking structures are passive, and they just lie around consuming only memory; but timer is not a passive structure. It's actively consuming CPU cycles, easily using up an entire core due to a nonchalant use of this wrapper function.

And therein lies the confusion. I think we should trade clarity over a technicality, and outright mention the more significant adverse effect, which is CPU consumption caused by a fast iterating loop using this wrapper function.

@rsc
Copy link
Contributor

rsc commented Nov 11, 2016

The docs already call out that leaking these is a potential problem you should avoid.
I'm sorry, but I don't think the exact consequences of the leak need to be spelled out.
They may vary from implementation to implementation.

@rsc rsc closed this as completed Nov 11, 2016
@golang golang locked and limited conversation to collaborators Nov 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants