onsi / ginkgo

A Modern Testing Framework for Go
http://onsi.github.io/ginkgo/
MIT License
8.27k stars 654 forks source link

watch consuming non-trivial CPU #370

Closed decibel closed 3 years ago

decibel commented 7 years ago

I've run into situations where ginkgo watch is consuming a fair amount of CPU just watching for changes (on the order of 10-15% on a new MacBook Pro). Unfortunately I just rebooted, so the 4 I have running right now are only using between 1.6 and 3.6% each (each of those is in a different branch of the same repository, all running in completely separate go environments).

It looks like the code simply checks for changes once a second; have you looked into using something like https://github.com/fsnotify/fsnotify?

BTW, ISTM that dependency handling is a weak spot in go. For example, before firing up ginkgo watch, I need to generate mocks, which happens via go:generate. Unfortunately that's a relatively time consuming process, and 98% of the time it's wasted effort because nothing has changed. And if something has changed, it's only for a few mocks. I suspect that taking the work you've done with package dependencies and breaking it out into a separate package would be helpful to others. Even better if it could be expanded to detect changes in (or monitor) individual constructs in a package.

onsi commented 7 years ago

hey @decibel - sorry for the delay getting back to you. when I implemented the polling approach I was a bit nervous about cpu usage but a few cursory tests showed it To Be Mostly OK™. with that said i can totally imagine it consuming as much as you're seeing, especially if you're running ~4+ instances of ginkgo watch.

IIRC at the time I was writing watch (maybe 3 years ago?) I had trouble getting fsnotify to actually work consistently. It would routinely miss events. Perhaps things are better now though? Would you be interested in trying to swap out the polling mechanism with fsnotify and submitting a PR?

onsi commented 7 years ago

I should add I had trouble with fsnotify on macOS. Not sure if things are in a better place now or not - do you know?

dudehook commented 7 years ago

Hey - I'm planning on modifying Ginkgo to use https://github.com/fsnotify/fsnotify for file watching. I use both Mac and Linux, and will try on both.

I believe I can modify the tracker which will respond to the fs notifications and update the structures you already have. Also, I'd move the timer into the tracker, so that the tracker can operate independently regardless of being a timed poller or a fsnotify subscriber.

Finally, I'd create an interface for the tracking stuff so that you use the interface and tell it if you want a poller or a notifier and it would look the same to the rest of the application.

Sound good?

onsi commented 7 years ago

👍 awesome!!!

On Aug 24, 2017, at 5:55 PM, David Hooker notifications@github.com wrote:

Hey - I'm planning on modifying Ginkgo to use https://github.com/fsnotify/fsnotify for file watching. I use both Mac and Linux, and will try on both.

I believe I can modify the tracker which will respond to the fs notifications and update the structures you already have. Also, I'd move the timer into the tracker, so that the tracker can operate independently regardless of being a timed poller or a fsnotify subscriber.

Finally, I'd create an interface for the tracking stuff so that you use the interface and tell it if you want a poller or a notifier and it would look the same to the rest of the application.

Sound good?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

dudehook commented 7 years ago

@onsi Where is "after_pr.sh"?

dudehook commented 6 years ago

@onsi I started work on this in my fork. Sent you an invite to that fork.

onsi commented 3 years ago

I'm working through the backlog of old Ginkgo issues - apologies as this issue is probably stale now. Sorry I never managed to circle back back in 2017. i've got much more time for Ginkgo these days and am working towards v2. I'm currently planning on sticking with the polling behavior of watch but may circle back and replace it with a notification based mechanism in a subsequent release.

I'm going to close this for now, but feel free to reopen it if you'd like.