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

Refactoring watch for performance #363

Merged
merged 4 commits into from
Mar 10, 2017
Merged

Refactoring watch for performance #363

merged 4 commits into from
Mar 10, 2017

Conversation

tanema
Copy link
Contributor

@tanema tanema commented Mar 8, 2017

fixes #349

After I refactored the watching event loop so that I could more accurately profile the cpu usage actually went down to peak 7%. This could be because I have moved a large amount of variables from anonymous functions into the watcher struct. This could the GC less like to take longer in figuring out if the variables are needed or not. This is only a guess because I am still not quite able to figure out what was causing the performance issues.

@tanema tanema requested a review from chrisbutcher March 8, 2017 21:47
Tim Anema added 2 commits March 9, 2017 09:53
…le when an event happens so that the loop isn't constantly looping
…d keep creating new go routines that would not end
@tanema
Copy link
Contributor Author

tanema commented Mar 9, 2017

I refactored the notify file so that the watch event loop no longer keeps ticking for the notify file. It will only create a go routine when an event happened to touch the notify file.

With this change I have seen peak CPU at 0.5%

Copy link
Contributor

@chrisbutcher chrisbutcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@tanema tanema merged commit faa2921 into master Mar 10, 2017
@tanema tanema deleted the watch_performance branch March 10, 2017 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

macOS high CPU usage on theme watch
2 participants