Re: JDK-8193445 Performance Test
Hi, I just wanted to clarify a few things about the fix. The test I showed hints at a potentially safer way to implement the fix for this problem. One would expect adding nodes 1 by 1 to the scene graph to be less efficient than adding them all at once, but it's far worse to add them all at once. The fix demonstrates that the test's performance problem comes from the reapplication of css, and is not necessarily the best fix for the problem, it's just the least impactful fix I could come up with, since the idea is that adding nodes one by one is safe already. Reapplying CSS to a Node but not its children could cause a problem if > there are styles in the parent or the parent's parents that affect the > children. It seems like bypassing children in reapplyCSS is bound to > cause a regression. > CSS does get applied to the children. The problem was that each parent would call scenesChanged, which would then call scenesChanged on all the children recursively. As they're bubbling out they call reapplyCSS on themselves which calls reapplyCss on themselves and all their children. Jonathan's solution was to only allow the node that was added to the scene graph to reapplyCSS to its children, thus applying css in a top down way. I suspect with some modifications it could work. One thing it didn't do was call reapplyCSS (different from reapplyCss) on every node. It only called reapplyCss on every node, and there is some additional logic in reapplyCSS. (The commit which undoes this change is here: https://github.com/javafxports/openjdk-jfx/commit/cd14f72776281a2384c50a35e618f1fb258c9430#diff-4a65018af7ebb15827ff1c67a3c29e86 ). In mine, reapplyCSS happens first and doesn't propagate down to the children. Instead, the scenesChanged method will trigger invalidatedScenes on all the children, and they will reapplyCSS for themselves. Basically each node calls reapplyCSS for itself, then propagates. In this way css is applied top down in a way similar to adding the nodes 1 by 1. Also, only this one case where nodes are added all at once is affected. Kevin has also mentioned https://bugs.openjdk.java.net/browse/JDK-8173301 as a way to mitigate the problem. I think something like that would be fine for this particular css performance issue, but there are several other issues. It would be nice to have a safer way to work on these issues so that they could get tackled. I think there are several options here. I just wanted to share some insights, and perhaps contribute a test or two. Kevin mentioned "We will need a performance regression test to measure the gain." Are there any performance tests like this already I could look at as an example? Dean On Wed, Nov 7, 2018 at 3:37 PM David Grieve wrote: > One of the dangers of mucking around with the CSS code is whether or not > the changes break things like popups, dialogs, menus. And whether or not > the change breaks inline styles versus attributes set in code, versus > stylesheets added to the scene/subscene/control, versus default > stylesheets. And whether or not the changes breaks skins for controls. > > Reapplying CSS to a Node but not its children could cause a problem if > there are styles in the parent or the parent's parents that affect the > children. It seems like bypassing children in reapplyCSS is bound to > cause a regression. > > Also, because a new scene root could affect styles, CSS is reapplied > after calling sceneChanged - your change puts it before, so I question > whether or not this will cause a regression. I think if you skip > reapplying CSS when a Node's scene has changed, then the children won't > get the correct styles, and this will affect layout. > > I haven't spent much time evaluating this change in detail, but I'm > doubtful that it won't cause regressions. > >
Re: JDK-8193445 Performance Test
My thoughts as well. I would love to see a safe robust fix for this
issue, but there is a very real possibility of a regression: We thought
we had a safe fix earlier and only later discovered the (multiple)
regressions a few months after it was released.
One possible way to improve performance is to allow applications to
manage when it is safe to skip applying CSS to certain nodes under app
control, as proposed in
https://bugs.openjdk.java.net/browse/JDK-8173301. I'm not sure that
would be as effective, but at least it would only affect apps that "opt
in" and only for those parts of the scene graph that are so designated.
I do like the idea of coming up with better tests that can be used to
validate any potential future fix.
-- Kevin
On 11/7/2018 5:37 AM, David Grieve wrote:
One of the dangers of mucking around with the CSS code is whether or
not the changes break things like popups, dialogs, menus. And whether
or not the change breaks inline styles versus attributes set in code,
versus stylesheets added to the scene/subscene/control, versus default
stylesheets. And whether or not the changes breaks skins for controls.
Reapplying CSS to a Node but not its children could cause a problem if
there are styles in the parent or the parent's parents that affect the
children. It seems like bypassing children in reapplyCSS is bound to
cause a regression.
Also, because a new scene root could affect styles, CSS is reapplied
after calling sceneChanged - your change puts it before, so I question
whether or not this will cause a regression. I think if you skip
reapplying CSS when a Node's scene has changed, then the children
won't get the correct styles, and this will affect layout.
I haven't spent much time evaluating this change in detail, but I'm
doubtful that it won't cause regressions.
On 11/7/18 2:57 AM, Dean Wookey wrote:
Hi,
I was going to ask if it was possible to reopen JDK-8151756 (
https://bugs.openjdk.java.net/browse/JDK-8151756) since it was fixed but
reverted in JDK-8183100
(https://bugs.openjdk.java.net/browse/JDK-8183100)
after causing several regressions. I only noticed now that a followup
bug
was created: JDK-8193445 which reopens the issue.
The code below demonstrates the problem where adding nodes to the scene
graph all at once performs exponentially slower than adding them one by
one. I get the following results with jfx11.0.1:
One by one: 138ms
All at once: 16704ms
I've made a potential fix, different to the one tried previously which
applies the css as if the nodes were being added one by one:
https://github.com/DeanWookey/openjdk-jfx/commit/65a1ed82bce262294f1969e9a12e1126ec8a1ec6
It passes the main tests, as well as the systemTest JDK8183100Test.java
from https://bugs.openjdk.java.net/browse/JDK-8193494.
This is probably not a suitable issue to work on for a first time
contributor, but perhaps I could work on a performance test if
someone can
point me in the direction of existing performance tests?
Dean
package applycsstest;
import javafx.application.Application;
import javafx.application.Platform;
import javafx.scene.Scene;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;
/**
*
* @author Dean
*/
public class ApplyCssTest extends Application {
@Override
public void start(Stage primaryStage) {
System.out.println("One by one: " + addToSceneOneByOne() +
"ms");
System.out.println("All at once: " + addToSceneAllAtOnce() +
"ms");
Platform.exit();
}
public static void main(String[] args) {
launch(args);
}
public long addToSceneOneByOne() {
StackPane root = new StackPane();
Scene scene = new Scene(root, 300, 250);
long start = System.currentTimeMillis();
StackPane firstChild = new StackPane();
root.getChildren().add(firstChild); //add to the scene graph
first
addNodesOneByOne(1000, firstChild); //then add children one
by one
return System.currentTimeMillis() - start;
}
public long addToSceneAllAtOnce() {
StackPane root = new StackPane();
Scene scene = new Scene(root, 300, 250);
long start = System.currentTimeMillis();
StackPane firstChild = new StackPane();
addNodesOneByOne(1000, firstChild); //build the node up, then
root.getChildren().add(firstChild); //add to scene graph all
at once
return System.currentTimeMillis() - start;
}
public void addNodesOneByOne(int numToAdd, StackPane parent) {
StackPane last = parent;
for (int i = 0; i < numToAdd; i++) {
StackPane curr = new StackPane();
last.getChildren().add(curr);
last = curr;
}
}
}
Re: JDK-8193445 Performance Test
> On Nov 7, 2018, at 8:37 AM, David Grieve wrote: > > ... > > Reapplying CSS to a Node but not its children could cause a problem if there > are styles in the parent or the parent's parents that affect the children. It > seems like bypassing children in reapplyCSS is bound to cause a regression. I agree that it is necessary to re-apply CSS to child nodes when the parent node CSS changes. The issue that I originally reported was that I could see that child nodes had CSS re-applied more than once. The number of times CSS is applied is currently proportional to how deep in the scene graph hierarchy the node is. When a subtree is added to the scene graph, I would expect that CSS can be reapplied to each node only once, so long as it is done in a top-down manner. I could be wrong, I’m not certain how rules relating to sibling nodes might invalidate that assumption. In HTML there is the concept of adjacent sibling combinator, e.g. “img + p" for paragraphs that come immediately after any image. I don’t think JavaFX has anything like that thoughts o top-down should work out. Scott
Re: Running JavaFX-11 applications from within Eclipse fails
I have a related problem when developing JavaFX in Eclipse and Win10 that
started in 11.
I created a modular project and in its build configuration (in Eclipse) I
added the JavaFX base and graphics projects (that point to rt\modules\...)
and OpenJDK11 to the module path.
In the module-info file I required the JavaFX modules and exported my
package.
Compilation is fine, but during runtime (with -Dprism.verbose=true) I get
an error:
Prism pipeline init order: d3d sw
Using Double Precision Marlin Rasterizer
Using dirty region optimizations
Not using texture mask for primitives
Not forcing power of 2 sizes for textures
Using hardware CLAMP_TO_ZERO mode
Opting in for HiDPI pixel scaling
Prism pipeline name = com.sun.prism.d3d.D3DPipeline
Loading D3D native library ...
// Error here: //
GraphicsPipeline.createPipeline failed for com.sun.prism.d3d.D3DPipeline
java.lang.UnsatisfiedLinkError: no prism_sw in java.library.path: [see
below]
at java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:2660)
at java.base/java.lang.Runtime.loadLibrary0(Runtime.java:829)
at java.base/java.lang.System.loadLibrary(System.java:1867)
at
javafx.graphics/com.sun.glass.utils.NativeLibLoader.loadLibraryInternal(NativeLibLoader.java:150)
at
javafx.graphics/com.sun.glass.utils.NativeLibLoader.loadLibrary(NativeLibLoader.java:52)
at
javafx.graphics/com.sun.prism.d3d.D3DPipeline.lambda$0(D3DPipeline.java:48)
at java.base/java.security.AccessController.doPrivileged(Native Method)
at
javafx.graphics/com.sun.prism.d3d.D3DPipeline.(D3DPipeline.java:44)
at java.base/java.lang.Class.forName0(Native Method)
at java.base/java.lang.Class.forName(Class.java:315)
at
javafx.graphics/com.sun.prism.GraphicsPipeline.createPipeline(GraphicsPipeline.java:187)
at
javafx.graphics/com.sun.javafx.tk.quantum.QuantumRenderer$PipelineRunnable.init(QuantumRenderer.java:91)
at
javafx.graphics/com.sun.javafx.tk.quantum.QuantumRenderer$PipelineRunnable.run(QuantumRenderer.java:124)
at java.base/java.lang.Thread.run(Thread.java:834)
*** Fallback to Prism SW pipeline
Prism pipeline name = com.sun.prism.sw.SWPipeline
GraphicsPipeline.createPipeline failed for com.sun.prism.sw.SWPipeline
java.lang.UnsatisfiedLinkError: no prism_sw in java.library.path: [see
below]
at java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:2660)
at java.base/java.lang.Runtime.loadLibrary0(Runtime.java:829)
at java.base/java.lang.System.loadLibrary(System.java:1867)
at
javafx.graphics/com.sun.glass.utils.NativeLibLoader.loadLibraryInternal(NativeLibLoader.java:150)
at
javafx.graphics/com.sun.glass.utils.NativeLibLoader.loadLibrary(NativeLibLoader.java:52)
at javafx.graphics/com.sun.prism.sw.SWPipeline.lambda$0(SWPipeline.java:42)
at java.base/java.security.AccessController.doPrivileged(Native Method)
at javafx.graphics/com.sun.prism.sw.SWPipeline.(SWPipeline.java:41)
at java.base/java.lang.Class.forName0(Native Method)
at java.base/java.lang.Class.forName(Class.java:315)
at
javafx.graphics/com.sun.prism.GraphicsPipeline.createPipeline(GraphicsPipeline.java:187)
at
javafx.graphics/com.sun.javafx.tk.quantum.QuantumRenderer$PipelineRunnable.init(QuantumRenderer.java:91)
at
javafx.graphics/com.sun.javafx.tk.quantum.QuantumRenderer$PipelineRunnable.run(QuantumRenderer.java:124)
at java.base/java.lang.Thread.run(Thread.java:834)
Graphics Device initialization failed for : d3d, sw
Error initializing QuantumRenderer: no suitable pipeline found
// ...
The paths listed at the end are those from %PATH% and none point to the
development modules. So, I added to the runtime VM args in the launch
configuration:
-Djava.library.path="...\rt\modules\javafx.graphics\bin"
and I also tried with
"...\rt\modules\javafx.graphics\bin\com\sun\prism\d3d" because this is
where com.sun.prism.d3d.D3DPipeline is.
I get the same error. Did anyone encounter this?
- Nir
On Sun, Nov 4, 2018 at 6:40 PM Tom Schindl
wrote:
> I think the more general problem is that they don‘t run on the module-path
> - in the m2e case this because the modules are transitive deps and those
> are not supported properly
>
> Tom
>
> Von meinem iPhone gesendet
>
> > Am 04.11.2018 um 16:17 schrieb José Pereda :
> >
> > I've just noticed that this issue happens not only with Maven but also
> with
> > Gradle projects (Gradle + Eclipse 2018-09 + Windows with Oracle JDK 1.8),
> > running gradle tasks from Eclipse.
> >
> > The same proposed workaround can be applied to the build file:
> >
> > run {
> >
> >systemProperty "java.library.path", "C:\tmp"
> >
> > }
> >
> >
> >
> >
> > On Mon, Oct 22, 2018 at 4:43 PM Kevin Rushforth <
> kevin.rushfo...@oracle.com>
> > wrote:
> >
> >>
> >>> Renaming the native libraries in JavaFX would probably solve this, but
> >> that
> >>> seems the wrong solution to me.
> >>
> >> Yes, it seems like a workaround rather than a fix...
> >>
> >> -- Kevin
> >>
> >>
> >>> On 10/21/2018 10:45 AM, Johan Vos wrote:
> >>> Hi Tom,
> >>>
> >>> Nice workaround, but what do you think needs to be done to fix it
Re: JDK-8193445 Performance Test
One of the dangers of mucking around with the CSS code is whether or not
the changes break things like popups, dialogs, menus. And whether or not
the change breaks inline styles versus attributes set in code, versus
stylesheets added to the scene/subscene/control, versus default
stylesheets. And whether or not the changes breaks skins for controls.
Reapplying CSS to a Node but not its children could cause a problem if
there are styles in the parent or the parent's parents that affect the
children. It seems like bypassing children in reapplyCSS is bound to
cause a regression.
Also, because a new scene root could affect styles, CSS is reapplied
after calling sceneChanged - your change puts it before, so I question
whether or not this will cause a regression. I think if you skip
reapplying CSS when a Node's scene has changed, then the children won't
get the correct styles, and this will affect layout.
I haven't spent much time evaluating this change in detail, but I'm
doubtful that it won't cause regressions.
On 11/7/18 2:57 AM, Dean Wookey wrote:
Hi,
I was going to ask if it was possible to reopen JDK-8151756 (
https://bugs.openjdk.java.net/browse/JDK-8151756) since it was fixed but
reverted in JDK-8183100 (https://bugs.openjdk.java.net/browse/JDK-8183100)
after causing several regressions. I only noticed now that a followup bug
was created: JDK-8193445 which reopens the issue.
The code below demonstrates the problem where adding nodes to the scene
graph all at once performs exponentially slower than adding them one by
one. I get the following results with jfx11.0.1:
One by one: 138ms
All at once: 16704ms
I've made a potential fix, different to the one tried previously which
applies the css as if the nodes were being added one by one:
https://github.com/DeanWookey/openjdk-jfx/commit/65a1ed82bce262294f1969e9a12e1126ec8a1ec6
It passes the main tests, as well as the systemTest JDK8183100Test.java
from https://bugs.openjdk.java.net/browse/JDK-8193494.
This is probably not a suitable issue to work on for a first time
contributor, but perhaps I could work on a performance test if someone can
point me in the direction of existing performance tests?
Dean
package applycsstest;
import javafx.application.Application;
import javafx.application.Platform;
import javafx.scene.Scene;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;
/**
*
* @author Dean
*/
public class ApplyCssTest extends Application {
@Override
public void start(Stage primaryStage) {
System.out.println("One by one: " + addToSceneOneByOne() + "ms");
System.out.println("All at once: " + addToSceneAllAtOnce() + "ms");
Platform.exit();
}
public static void main(String[] args) {
launch(args);
}
public long addToSceneOneByOne() {
StackPane root = new StackPane();
Scene scene = new Scene(root, 300, 250);
long start = System.currentTimeMillis();
StackPane firstChild = new StackPane();
root.getChildren().add(firstChild); //add to the scene graph first
addNodesOneByOne(1000, firstChild); //then add children one by one
return System.currentTimeMillis() - start;
}
public long addToSceneAllAtOnce() {
StackPane root = new StackPane();
Scene scene = new Scene(root, 300, 250);
long start = System.currentTimeMillis();
StackPane firstChild = new StackPane();
addNodesOneByOne(1000, firstChild); //build the node up, then
root.getChildren().add(firstChild); //add to scene graph all at once
return System.currentTimeMillis() - start;
}
public void addNodesOneByOne(int numToAdd, StackPane parent) {
StackPane last = parent;
for (int i = 0; i < numToAdd; i++) {
StackPane curr = new StackPane();
last.getChildren().add(curr);
last = curr;
}
}
}
JDK-8193445 Performance Test
Hi,
I was going to ask if it was possible to reopen JDK-8151756 (
https://bugs.openjdk.java.net/browse/JDK-8151756) since it was fixed but
reverted in JDK-8183100 (https://bugs.openjdk.java.net/browse/JDK-8183100)
after causing several regressions. I only noticed now that a followup bug
was created: JDK-8193445 which reopens the issue.
The code below demonstrates the problem where adding nodes to the scene
graph all at once performs exponentially slower than adding them one by
one. I get the following results with jfx11.0.1:
One by one: 138ms
All at once: 16704ms
I've made a potential fix, different to the one tried previously which
applies the css as if the nodes were being added one by one:
https://github.com/DeanWookey/openjdk-jfx/commit/65a1ed82bce262294f1969e9a12e1126ec8a1ec6
It passes the main tests, as well as the systemTest JDK8183100Test.java
from https://bugs.openjdk.java.net/browse/JDK-8193494.
This is probably not a suitable issue to work on for a first time
contributor, but perhaps I could work on a performance test if someone can
point me in the direction of existing performance tests?
Dean
package applycsstest;
import javafx.application.Application;
import javafx.application.Platform;
import javafx.scene.Scene;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;
/**
*
* @author Dean
*/
public class ApplyCssTest extends Application {
@Override
public void start(Stage primaryStage) {
System.out.println("One by one: " + addToSceneOneByOne() + "ms");
System.out.println("All at once: " + addToSceneAllAtOnce() + "ms");
Platform.exit();
}
public static void main(String[] args) {
launch(args);
}
public long addToSceneOneByOne() {
StackPane root = new StackPane();
Scene scene = new Scene(root, 300, 250);
long start = System.currentTimeMillis();
StackPane firstChild = new StackPane();
root.getChildren().add(firstChild); //add to the scene graph first
addNodesOneByOne(1000, firstChild); //then add children one by one
return System.currentTimeMillis() - start;
}
public long addToSceneAllAtOnce() {
StackPane root = new StackPane();
Scene scene = new Scene(root, 300, 250);
long start = System.currentTimeMillis();
StackPane firstChild = new StackPane();
addNodesOneByOne(1000, firstChild); //build the node up, then
root.getChildren().add(firstChild); //add to scene graph all at once
return System.currentTimeMillis() - start;
}
public void addNodesOneByOne(int numToAdd, StackPane parent) {
StackPane last = parent;
for (int i = 0; i < numToAdd; i++) {
StackPane curr = new StackPane();
last.getChildren().add(curr);
last = curr;
}
}
}
