<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>I noticed the copy/paste I did wasn't great, you don't need the
`future` variable or code that manipulates it (I had a different
version before, but this works quite well for now)</p>
<p>--John<br>
</p>
<div class="moz-cite-prefix">On 08/08/2024 12:47, Thiago Milczarek
Sayão wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAAP_wu=TuDzUCLWKRM83Rr_FQPPxK4U4zmF2wFx-LyZB8YarDQ@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<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" moz-do-not-send="true"
class="moz-txt-link-freetext">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">
<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>
<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" moz-do-not-send="true"
class="moz-txt-link-freetext">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" moz-do-not-send="true"
class="moz-txt-link-freetext">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>
</blockquote>
</body>
</html>