-
Notifications
You must be signed in to change notification settings - Fork 80
Automatic event loop start/stop when not showing anything #607
Conversation
Codecov Report
@@ Coverage Diff @@
## master #607 +/- ##
==========================================
- Coverage 56.80% 55.80% -1.00%
==========================================
Files 32 32
Lines 2669 2238 -431
==========================================
- Hits 1516 1249 -267
+ Misses 1153 989 -164
Continue to review full report at Codecov.
|
Thanks Ian for pushing this forward, this is much appreciated!!! I would like to ping @jpsamaroo @tkf @vtjnash @JeffBezanson @chriselrod: Can anybody from you give a perspective on this PR or what the route forward for proper Gtk mainloop integration with Julia Threads/Tasks is? It is really a problem that one cannot profile multi-threading applications with |
@IanButterworth: Ping me once you think this is ready and can be merged + release. A workaround a certainly better than nothing right now. |
c070ce6
to
4a07406
Compare
I'm happy with this now. Tests pass locally. Windows open and behave correctly, including with ProfileView, with no downstream code changes. @tknopp one question is whether there are any more calls to display widgets/windows that need the |
4a07406
to
cb15a81
Compare
@tknopp PR JuliaLang/julia#42302 implements threadpools, which would allow Gtk.jl to request that some subset of its tasks get scheduled in an isolated threadpool. Those tasks would be isolated from the rest of the Julia runtime (and thus achieve lower latency even under higher compute load from unrelated code). If no additional threadpools are configured, the tasks will instead remain in the primary threadpool. I'm implementing the Julia API for threadpools now, and will let you know once the PR is ready for testing! |
Perhaps even when the threadpools work is released in base, this PR might remain a good idea? Someone who just wants to profile their multithreaded system with Gtk probably won't want Gtk to swallow a thread while the profile is being collected? |
I do think this PR is a good idea even with thread pools (which I also think are a great idea). When I don't have an open Gtk window, I definitely want all cores to contribute as much as possible to e.g. a multithreaded loop. This means that the Gtk main loop shouldn't be active at all unless it is actually needed, which is guaranteed by this PR. |
I did find what appears to be a redundant show in ImageView.jl that made this PR think something was open after all windows had been closed though. Not quite sure what's happening there JuliaImages/ImageView.jl#259 But I don't think that needs to hold this back. There may be tweak PRs following this to catch all show entry and exit points. I wonder whether the Gtk show methods return whether they actually caused anything to appear.. that way the handler could be conditionally set. |
Ok. Fixed so that things like JuliaImages/ImageView.jl#259 aren't needed. Putting it behind the |
So I am for merging this even if thread pools come in the future, which will not before 1.8 anyway. @IanButterworth: Is this good to go from your side? |
Indeed. Good to go as a non-breaking change, and I'll watch out for downstream bugs. |
This is a great idea, thanks so much for fixing this vexing problem! |
Fixed buttons during initial startup not doing anything. Issue introduced by JuliaGraphics/Gtk.jl#607. Added color names to room window color field tooltip. Fixed left clicking with circle tool not working.
As reported in #503 & JuliaLang/julia#35552 loading Gtk in a julia session basically ruins the performance of the session from there-on out, especially threading.
Fixing the core issue appears to be intrinsic to how julia runs threads, and might take a while to fix(?)
So... this PR:
Gtk.enable_eventloop(b::Bool)
for start/stopping the GLib event loop, which prevents blocking other julia tasks/threadsshow
orshowall
so that rendering always starts when something is shownshow
-ed widgets have been destroyedGtk.AUTO_IDLE[] = false
Big caveat:I followed this for guidance on whatuv_prepare
anduv_check
should return to set idle state, but it's all effectively just setting the timeout to 0, resulting in a cpu thread at 100%. It would clearly be better to start/stop the main thread, so it would be good to implement what @StefanMathis demoed here #503 (comment)Update: This now fully starts/stops the eventloop, so no cpu penalty.
Example
Currently execution is slowed from the moment Gtk is launched
With this PR, execution is only slowed when the Gtk window is open and static
Notes