Warn on windows

[SimonDanisch]

Can we please, please leave this warning? Debugging this problem can waste days of frustrating debugging, even for experienced Julia developers, as you can see here: https://discourse.julialang.org/t/serious-issue-julia-language-keyboard-lag-typing-very-slow/32081/23
A package, which turns calculating a simple spectrogram from 0.02s to 1.7s (it's actually not as bad anymore, it was 50s before 1.1.0) just by using it anywhere in the dependency stack definitely needs to be warned about.

(v1.3) pkg> st Gtk
    Status `C:\Users\sdani\.julia\environments\v1.3\Project.toml`
  [4c0ca9eb] Gtk v1.1.0
julia> versioninfo()
Julia Version 1.3.0
Commit 46ce4d7933 (2019-11-26 06:09 UTC)
Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.1 (ORCJIT, skylake)
Environment:
  JULIA_NUM_THREADS = 8

julia> using DSP
julia> @time stft(rand(300_000));
  0.022403 seconds (13.61 k allocations: 8.404 MiB)
julia> using Gtk
julia> @time stft(rand(300_000));
  1.736896 seconds (533.13 k allocations: 33.170 MiB)

[tknopp]

Can we have a more neutral warning. "Buggy" has negative meaning IMHO.
Note that the previous PR needed to be dropped since it was not compatible with the Artifact branch of Eliot.

[codecov]

Codecov Report

Merging #467 into master will increase coverage by 0.14%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #467      +/-   ##
==========================================
+ Coverage    49.5%   49.64%   +0.14%     
==========================================
  Files          32       32              
  Lines        2135     2137       +2     
==========================================
+ Hits         1057     1061       +4     
+ Misses       1078     1076       -2

Impacted Files	Coverage Δ
src/Gtk.jl	91.3% <100%> (+0.82%)	⬆️
src/GLib/gtype.jl	77.72% <0%> (+0.99%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5347a35...78f0f0c. Read the comment docs.

[tknopp]

ok, thanks.

[timholy]

I assume there is no fix for this on the horizon?

[giordano]

I seem to remember that Jameson suggested to replace a number with another number somewhere, but that's lost in slack history - or I completely dreamt this.

[SimonDanisch]

Maybe this has happened, since the problem got ~30x better (it's still immediately noticeable though :( )

[timholy]

This seems likely to be due to polling, so I'm guessing some constants here or here might be relevant.
The second controls the flags that get polled for (https://developer.gnome.org/glib/unstable/glib-The-Main-Event-Loop.html#GPollFD), and became more restrictive in #447 (from "all" to just G_IO_IN).

In the first one, maybe try changing that 100 to 1000 and see if it gets better? I can't easily test this myself, I do have a virtualbox Windows install (if it can be replicated on that platform) but I'm stymied currently by #461.

[giordano]

Yes, I think it was one of those numbers.

but I'm stymied currently by #461.

Doesn't adding Sys.BINDIR to the PATH work around the issue?

[timholy]

That fixes it, thanks.

I can replicate the REPL lag but not the stft demo in the OP. Maybe an issue of being on VirtualBox?

Empirically, changing the 100 on this line to 10 seems to improve the REPL lag. IIUC, this just affects how long it's willing to wait while polling. But I don't pretend to fully understand how this is working, and I worry this could cause it to drop events on Windows. Would appreciate it if others who know more can chime in (@vtjnash, @jonathanBieler). FWIW, the tests passed for me.

[jonathanBieler]

My understanding (based on this comment) of the issue is that in order not to block the REPL with it's main loop Gtk.jl does some funny business with julia Tasks, which doesn't work very well on Windows.

I'm not sure if that's a fundamental issue or it can be fixed by tweaking things up a bit. But maybe with the new thread system all this could be redesigned (run Gtk main loop in a thread instead of a task ?). The issue is that we need someone that understand Julia's tasks, threads, Gtk and has time to look at it, which is probably not many people, if any.

[timholy]

This warning seems to cause a great deal of stress for some people: https://discourse.julialang.org/t/image-in-repl-does-not-correct/46359/5. Not sure what to do here.

[tknopp]

Remove the warning?

[tknopp]

And if you changes from 100 to 10 and see an improvement, may just change that?

[SimonDanisch]

I just tested the newest Gtk on 1.5.1 and the REPL slowdown is still significant, and will still be very confusing if a random Package has a silent dependency on Gtk.
The good news is, that the DSP benchmark I showed in the PR for the warning now yields the same results with & without Gtk loaded.

[timholy]

Let's try dropping that constant but leave the warning.

I tried naively putting the event loop in a thread:

diff --git a/src/Gtk.jl b/src/Gtk.jl
index f633bda..a89ff40 100644
--- a/src/Gtk.jl
+++ b/src/Gtk.jl
@@ -38,6 +38,8 @@ using Cairo
 import Cairo: destroy
 using Serialization
 
+using ThreadPools
+
 const Index{I<:Integer} = Union{I, AbstractVector{I}}
 
 export GAccessor
@@ -138,7 +140,11 @@ function __init__()
     # if g_main_depth > 0, a glib main-loop is already running,
     # so we don't need to start a new one
     if ccall((:g_main_depth, GLib.libglib), Cint, ()) == 0
-        global gtk_main_task = schedule(Task(gtk_main))
+        if Threads.nthreads() > 1
+            global gtk_main_task = @tspawnat Threads.nthreads() gtk_main()
+        else
+            global gtk_main_task = schedule(Task(gtk_main))
+        end
     end
 end

but I got deadlocks. I do not plan to dig into Gtk internals enough to understand how to fix that, sorry.

[timholy]

@SimonDanisch, can you try changing 100 to 10 here?

Gtk.jl/src/GLib/signals.jl

Line 300 in d7b1ef8

tmout_min::Cint = (uv_pollfd::_GPollFD).fd == -1 ? 100 : 5000

[timholy]

I'm going to submit that change but let's give Simon a chance to test before merging.