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:
/**
* Adds a slow event warning whenever an event takes more than 10 msto
process. Note
* that time spent in nested event loops cannot be properly taken into
account as time
* spent in nested event loops will be part of the event that triggered
it giving false
* positives. In order for this time to be accurately reflected, the
methods to enter
* a nested event loop in this class should be used instead of the ones
in {@link Platform}.
*
* @paramscene a Scene to which to add the slow event warning detection,
cannot be null
*/
publicstaticvoidaddSlowEventWarning(Scene scene) {
finalEventDispatcher eventDispatcher = scene.getEventDispatcher();
scene.setEventDispatcher(newEventDispatcher() {
privateScheduledFuture<StackTraceElement[]> future;
@Override
publicEvent dispatchEvent(Event event, EventDispatchChain tail) {
if(future!= null) {
future.cancel(false);
}
longstartTime = System.currentTimeMillis();
TIME_TRACKER.enterNested(startTime); // nesting can happen in two ways,
an event triggering another event, or when a nested event loop is entered
Event returnedEvent = eventDispatcher.dispatchEvent(event, tail);
longendTime = System.currentTimeMillis();
longtimeSpentInNested = TIME_TRACKER.exitNested(endTime);
if(timeSpentInNested > 10) {
longtotal = endTime - startTime;
LOGGER.warning("Slow Event (self/total: "+ timeSpentInNested + "/"+
total + " ms @ level "+ TIME_TRACKER.getCurrentLevel() + "): "+ event);
}
returnreturnedEvent;
}
});
}
--John
On 05/08/2024 17:17, Thiago Milczarek Sayão wrote:
Hi,
Interesting idea. We have this problem specially when Junior
developers touch the code.
The other way around would be nice too - if some I/O task executes on
the FX thread.
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.
-- Thiago.
Em seg., 5 de ago. de 2024 às 11:59, Kevin Rushforth
<kevin.rushfo...@oracle.com> escreveu:
Wouldn't it be better to implement this check in assert to avoid
any impact in production?
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.
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.
The things to consider would be:
1. What is the performance hit of doing this check on the setting
of every property?
2. What is the effect on bound properties?
3. How intrusive is it in the code?
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?
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.
-- Kevin
On 8/4/2024 8:40 PM, quizynox wrote:
Hello,
Wouldn't it be better to implement this check in assert to avoid
any impact in production?
пн, 5 авг. 2024 г. в 03:30, John Hendrikx <john.hendr...@gmail.com>:
Hi list,
I know of quite some bugs and users that have been bitten by the
threading model used by JavaFX. Basically, anything directly or
indirectly linked to an active Scene must be accessed on the
FX thread.
However, as FX also allows manipulating nodes and properties
before
they're displayed, there can be no "hard" check everywhere to
ensure we
are on the FX thread (specifically, in properties).
Now, I think this situation is annoying, as a simple mistake
where a
Platform.runLater wrapper was forgotten usually results in
programs
operating mostly flawlessly, but then fail in mysterious and
random and
hard to reproduce ways. The blame is often put on FX as the
resulting
exceptions will almost never show the user code which was the
actual
culprit. It can result in FX being perceived as unstable or
buggy.
So I've been thinking if there isn't something we can do to
detect these
bugs originating from user code much earlier, similar to the
`ConcurrentModificationException` the collection classes do when
accessed in nested or concurrent contexts.
I think it may be possible to have properties check whether
they're part
of an active scene without too much of an performance impact,
possibly
even behind a switch. It would work like this:
Properties involved with Nodes will have an associated bean
instance
(`getBean`). This is an object, but we could check here if this
instance implements an interface:
if (getBean() instanceof MayBePartOfSceneGraph x) {
if (x.isPartOfActiveScene() && !isOnFxThread()) {
throw new IllegalStateException("Property
must only be
used from the FX Application Thread");
}
}
This check could be done on every set of the property, and
potentially
on every get as well. It should be relatively cheap, but
will expose
problematic code patterns at a much earlier stage. There's a
chance
that this will "break" some programs that seemed to be behaving
correctly as well, so we may want to put it behind a switch
until such
programs (or libraries) can be fixed.
What do you all think?
--John
(*) Names of methods/interfaces are only used for
illustration purposes,
we can think of good names if this moves forward.
