x/tools/cmd/godoc consumes unreasonable amounts of CPU power

[the42]

When godoc starts it re-indexes files every 5 minutes, even if there are no changes. A
better behaviour would be to re-index upon changes signaled by ionotify

What happened?
godoc consumes every 5 minutes unreasonable amounts of CPU power

What should have happened instead?
Only re-index files after a relevant file has changed

[gopherbot]

Comment 1 by bgarcia@golang.org:

Labels changed: added repo-tools.

Owner changed to bgarcia@golang.org.

[rsc]

Comment 2:

Labels changed: added release-none.

Status changed to Accepted.

[agnivade]

It's not a simple slam dunk. There are couple of things to consider here -

1. We are trading off CPU for memory here. It is a fair tradeoff but should be considered nevertheless. Each inotify watch takes the size of 1 inode, which is around 1kB (source). So, for around 1000 directories, we will consume 1MB of memory.

2. There is an inherent limit specified by the OS to how many watches can be created. On linux, it is under /proc/sys/fs/inotify/max_user_watches. In my system, the value is 8192.
   Usually your $GOPATH would contain thousands of directories. In my machine, the no. of directories excluding .git under $GOPATH are 5473. An user can easily exceed this limit. In that case, the limit needs to be manually increased. Is that a good design ? Something to think upon.

But I agree that rebuilding the index unnecessarily even when nothing has changed, is worse off than this. I will prepare a CL.

/cc @andybons, @rsc - Thoughts ?

[andybons]

Another alternative--though I'm not saying it's a good one--is to allow a flag to specify how often to index or to disable reindexing entirely. It's unclear what's better here. Whether to add yet another flag or increase the complexity of the code by adding inotify watches. The other issue with watches is that they're platform-dependent. On macOS you have to use the FSEvents API and I have no idea what you have to do on Windows.

[agnivade]

It is already behind a flag called "index", which is false by default. And the index_interval is also behind a flag (default of 15 mins). So yes, you have to knowingly add a flag to enable indexing.

Secondly, I plan to use https://github.com/fsnotify/fsnotify which is fully cross-platform including Windows. Check out the repo.

But if you are satisfied with the flag solution, then I believe this issue can be closed.

[andybons]

Then let’s close this. If it comes up again we can revisit.

…

On Wed, Jan 24, 2018 at 10:43 AM Agniva De Sarker ***@***.***> wrote: It is *already* behind a flag called "index", which is false by default. And the index_interval is also behind a flag (default of 15 mins) So yes, you have to knowingly add a flag to enable indexing. Secondly, I plan to use https://github.com/fsnotify/fsnotify which is fully cross-platform including Windows. Check out the repo. But if you are satisfied with the flag solution, then I believe this issue can be closed. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7524 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAWSlCAmpj0pDD8NutP62gUCz4IxToqJks5tN0-cgaJpZM4RrA51> .

[agnivade]

Great 👍

[agnivade]

@andybons - I think you forgot to close this 😄