<div dir="ltr">Nice! I will steal it if you don't mind.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Em qua., 7 de ago. de 2024 às 15:55, John Hendrikx <<a href="mailto:john.hendrikx@gmail.com">john.hendrikx@gmail.com</a>> escreveu:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><u></u>
<div>
<p>For detecting long running tasks on the FX thread, there are some
other options which you can do as a user (but perhaps we can
support it directly within FX). I use this kind of code to detect
long running things on the FX thread:</p>
<div style="background-color:rgb(255,255,255);padding:0px 0px 0px 2px">
<div style="color:rgb(0,0,0);background-color:rgb(255,255,255);font-family:Consolas;font-size:11pt;white-space:pre-wrap"><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(63,95,191)">/**</span></p><p style="margin:0px"><span style="color:rgb(63,95,191)"> * Adds a slow event warning whenever an event takes more than 10 </span><span style="color:rgb(63,95,191);text-decoration:underline wavy rgb(255,128,64)">ms</span><span style="color:rgb(63,95,191)"> to process. Note</span></p><p style="margin:0px"><span style="color:rgb(63,95,191)"> * that time spent in nested event loops cannot be properly taken into account as time</span></p><p style="margin:0px"><span style="color:rgb(63,95,191)"> * spent in nested event loops will be part of the event that triggered it giving false</span></p><p style="margin:0px"><span style="color:rgb(63,95,191)"> * positives. In order for this time to be accurately reflected, the methods to enter</span></p><p style="margin:0px"><span style="color:rgb(63,95,191)"> * a nested event loop in this class should be used instead of the ones in </span><span style="color:rgb(63,63,191)">{@link Platform}</span><span style="color:rgb(63,95,191)">.</span></p><p style="margin:0px"><span style="color:rgb(63,95,191)"> *</span></p><p style="margin:0px"><span style="color:rgb(63,95,191)"> * </span><span style="color:rgb(127,159,191);font-weight:bold">@param</span><span style="color:rgb(63,95,191)"> scene a Scene to which to add the slow event warning detection, cannot be null</span></p><p style="margin:0px"><span style="color:rgb(63,95,191)"> */</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">public</span><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">static</span><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">void</span><span style="color:rgb(0,0,0)"> </span><span style="text-decoration:underline solid rgb(0,102,204);color:rgb(0,102,204)">addSlowEventWarning</span><span style="color:rgb(0,0,0)">(Scene scene) {</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">final</span><span style="color:rgb(0,0,0)"> EventDispatcher eventDispatcher = scene.getEventDispatcher();</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> scene.setEventDispatcher(</span><span style="color:rgb(0,0,160);font-weight:bold">new</span><span style="color:rgb(0,0,0)"> EventDispatcher() {</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">private</span><span style="color:rgb(0,0,0)"> ScheduledFuture<StackTraceElement[]> </span><span style="color:rgb(0,0,192)">future</span><span style="color:rgb(0,0,0)">;</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(100,100,100)">@Override</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">public</span><span style="color:rgb(0,0,0)"> Event dispatchEvent(Event event, EventDispatchChain tail) {</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">if</span><span style="color:rgb(0,0,0)">(</span><span style="color:rgb(0,0,192)">future</span><span style="color:rgb(0,0,0)"> != </span><span style="color:rgb(0,0,160);font-weight:bold">null</span><span style="color:rgb(0,0,0)">) {</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,192)">future</span><span style="color:rgb(0,0,0)">.cancel(</span><span style="color:rgb(0,0,160);font-weight:bold">false</span><span style="color:rgb(0,0,0)">);</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> }</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">long</span><span style="color:rgb(0,0,0)"> startTime = System.</span><span style="color:rgb(0,0,0);font-style:italic">currentTimeMillis</span><span style="color:rgb(0,0,0)">();</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,192)">TIME_TRACKER</span><span style="color:rgb(0,0,0)">.enterNested(startTime); </span><span style="color:rgb(63,127,95)">// nesting can happen in two ways, an event triggering another event, or when a nested event loop is entered</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> Event returnedEvent = eventDispatcher.dispatchEvent(event, tail);</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">long</span><span style="color:rgb(0,0,0)"> endTime = System.</span><span style="color:rgb(0,0,0);font-style:italic">currentTimeMillis</span><span style="color:rgb(0,0,0)">();</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">long</span><span style="color:rgb(0,0,0)"> timeSpentInNested = </span><span style="color:rgb(0,0,192)">TIME_TRACKER</span><span style="color:rgb(0,0,0)">.exitNested(endTime);</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">if</span><span style="color:rgb(0,0,0)">(timeSpentInNested > 10) {</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,160);font-weight:bold">long</span><span style="color:rgb(0,0,0)"> total = endTime - startTime;</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(0,0,192)">LOGGER</span><span style="color:rgb(0,0,0)">.warning(</span><span style="color:rgb(42,0,255)">"Slow Event (self/total: "</span><span style="color:rgb(0,0,0)"> + timeSpentInNested + </span><span style="color:rgb(42,0,255)">"/"</span><span style="color:rgb(0,0,0)"> + total + </span><span style="color:rgb(42,0,255)">" ms @ level "</span><span style="color:rgb(0,0,0)"> + </span><span style="color:rgb(0,0,192)">TIME_TRACKER</span><span style="color:rgb(0,0,0)">.getCurrentLevel() + </span><span style="color:rgb(42,0,255)">"): "</span><span style="color:rgb(0,0,0)"> + event);</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> }</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> </span><span style="color:rgb(127,0,85);font-weight:bold">return</span><span style="color:rgb(0,0,0)"> returnedEvent;</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> }</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> });</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)"> }</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)">
</span></p><p style="margin:0px"><span style="color:rgb(0,0,0)">--John
</span></p></div>
</div>
<p></p>
<div>On 05/08/2024 17:17, Thiago Milczarek
Sayão wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">Hi,
<div><br>
</div>
<div>Interesting idea. We have this problem specially when
Junior developers touch the code.</div>
<div><br>
</div>
<div>The other way around would be nice too - if some I/O task
executes on the FX thread.</div>
<div><br>
</div>
<div>This can make the OS think the application hanged and
offer to kill it, since it won't respond to "pings". And I/O
tasks processing time may vary between installations. Also
causes "white screens" since it blocks painting.</div>
<div><br>
</div>
<div>-- Thiago.</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">Em seg., 5 de ago. de 2024
às 11:59, Kevin Rushforth <<a href="mailto:kevin.rushforth@oracle.com" target="_blank">kevin.rushforth@oracle.com</a>>
escreveu:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<blockquote type="cite">Wouldn't it be better to implement
this check in assert to avoid any impact in production?</blockquote>
<br>
No. Using an assert in a case like this is an
anti-pattern. A call to assert in a library such as JavaFX
is only appropriate for checking an invariant in internal
logic. If we are going to go down this route of doing a
thread check when mutating properties of "live" nodes, we
will throw the same IllegalStateException that is
currently thrown by some methods on Stage and Scene.<br>
<br>
As for the proposal itself, adding this check is an
interesting idea. We considered doing this back in the JDK
7 (JavaFX 2) time frame, but decided not to pursue it
then. I think the idea is worth further discussion. I
would limit any thread checking to setting the property.
It would be too restrictive (and largely unnecessary) to
prevent reading a property from the application thread.<br>
<br>
The things to consider would be:<br>
<br>
1. What is the performance hit of doing this check on the
setting of every property?<br>
2. What is the effect on bound properties?<br>
3. How intrusive is it in the code?<br>
4. Should we add a property to enable / disable the thread
check, possibly a three- or four-valued property
(allow|warn|debug?|deny), as was recently done in JEP 471
for sun.misc.Unsafe memory access methods. If so, what
should the default be?<br>
<br>
My quick take is that if this can be done in a minimally
intrusive manner with low overhead, we should consider
pursing this. As for 4, my preference would be to add a
three- or four-valued system property to control the
check, with "warn" as the default initially, changing the
default to "disallow" in a subsequent version. This would,
of course, require a lot of testing.<br>
<br>
-- Kevin<br>
<br>
<br>
<div>On 8/4/2024 8:40 PM, quizynox wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hello,<br>
<br>
Wouldn't it be better to implement this check in
assert to avoid any impact in production?</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">пн, 5 авг. 2024 г. в
03:30, John Hendrikx <<a href="mailto:john.hendrikx@gmail.com" target="_blank">john.hendrikx@gmail.com</a>>:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi
list,<br>
<br>
I know of quite some bugs and users that have been
bitten by the <br>
threading model used by JavaFX. Basically, anything
directly or <br>
indirectly linked to an active Scene must be
accessed on the FX thread. <br>
However, as FX also allows manipulating nodes and
properties before <br>
they're displayed, there can be no "hard" check
everywhere to ensure we <br>
are on the FX thread (specifically, in properties).<br>
<br>
Now, I think this situation is annoying, as a simple
mistake where a <br>
Platform.runLater wrapper was forgotten usually
results in programs <br>
operating mostly flawlessly, but then fail in
mysterious and random and <br>
hard to reproduce ways. The blame is often put on
FX as the resulting <br>
exceptions will almost never show the user code
which was the actual <br>
culprit. It can result in FX being perceived as
unstable or buggy.<br>
<br>
So I've been thinking if there isn't something we
can do to detect these <br>
bugs originating from user code much earlier,
similar to the <br>
`ConcurrentModificationException` the collection
classes do when <br>
accessed in nested or concurrent contexts.<br>
<br>
I think it may be possible to have properties check
whether they're part <br>
of an active scene without too much of an
performance impact, possibly <br>
even behind a switch. It would work like this:<br>
<br>
Properties involved with Nodes will have an
associated bean instance <br>
(`getBean`). This is an object, but we could check
here if this <br>
instance implements an interface:<br>
<br>
if (getBean() instanceof MayBePartOfSceneGraph
x) {<br>
if (x.isPartOfActiveScene() &&
!isOnFxThread()) {<br>
throw new
IllegalStateException("Property must only be <br>
used from the FX Application Thread");<br>
}<br>
}<br>
<br>
This check could be done on every set of the
property, and potentially <br>
on every get as well. It should be relatively
cheap, but will expose <br>
problematic code patterns at a much earlier stage.
There's a chance <br>
that this will "break" some programs that seemed to
be behaving <br>
correctly as well, so we may want to put it behind a
switch until such <br>
programs (or libraries) can be fixed.<br>
<br>
What do you all think?<br>
<br>
--John<br>
<br>
(*) Names of methods/interfaces are only used for
illustration purposes, <br>
we can think of good names if this moves forward.<br>
<br>
</blockquote>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
</div>
</blockquote>
</div>
</blockquote></div>