Bug ID: JDK-8090848 FXMLLoader should be able to cache imports and properties between to load() calls

Other
tbdUnresolved

so, is there any update on this yet? It has been a long time.
27-10-2014
Reopening the issue as template loading API has been reverted until a better solution is found.
04-10-2013
It looks like this issue is open only because of RT-28121. It is enough to track the problem on one place. Switching back to Fixed.
18-09-2013
I reopened this issue like Nicolas suggested because I didn't feel like my previous comment was noticed and the other issue is forgotten.
19-07-2013
pretty cool if we could load dynamically fxml file(s) and then a way exists to avoid reloading innecessary
18-06-2013
Hello Nicolas, I already opened a new issue a while ago with a more sophisticated SCCE: RT-28121 One of your team members concluded that this issue cannot be fixed right now.
30-04-2013
Sebastian, please re-open the issue or file a new issue if you think here is a bug
29-04-2013
I tested this new feature with the current Java 8 EAP (b74) and I liked it. However, I noticed that the fxml that is used as a template is not allowed to have a controller set: ========== Box.fxml ============ <?xml version="1.0" encoding="utf-8"?> <?import java.lang.?> <?import javafx.scene.?> <?import javafx.scene.control.?> <?import javafx.scene.shape.?> <Group xmlns:fx="http://javafx.com/fxml" fx:controller="template.SampleController" fx:id="root"> <children> <Rectangle layoutX="0" layoutY="0" width="75" height="50" fill="green"/> <Label layoutX="0" layoutY="20" text="init"/> </children> </Group> ========== SampleController.java =========== public class SampleController { } ========== SampleApplication.java ========= import javafx.application.Application; import javafx.fxml.FXMLLoader; import javafx.scene.Group; import javafx.scene.Scene; import javafx.scene.control.Label; import javafx.stage.Stage; public class SampleApplication extends Application { public static void main(String[] args) { launch(args); } @Override public void start(Stage primaryStage) throws Exception { FXMLLoader loader = new FXMLLoader(); loader.setLocation(getClass().getResource("Box.fxml")); loader.setTemplate(true); Group box1 = (Group) loader.load(); setStuff(box1, "Third", 300, 50); Group box2 = (Group) loader.load(); setStuff(box2, "Second", 200, 50); Group box3 = (Group) loader.load(); setStuff(box3, "First", 100, 50); Group root = new Group(); root.getChildren().addAll(box1, box2, box3); Scene scene = new Scene(root, 500, 500); primaryStage.setScene(scene); primaryStage.show(); } private void setStuff(Group box, String text, int x, int y) { Label label = (Label) box.getChildren().get(1); label.setText(text); box.setLayoutX(x); box.setLayoutY(y); } } ======================== throws: Controller value already specified. /C:/Users/rheinnecker/IdeaProjects/JavaFX%20Tests/out/production/JavaFX%20Tests/template/Box.fxml:10 at javafx.fxml.FXMLLoader$ValueElement.processAttribute(FXMLLoader.java:752) at javafx.fxml.FXMLLoader$InstanceDeclarationElement.processAttribute(FXMLLoader.java:809) at javafx.fxml.FXMLLoader$Element.processStartElement(FXMLLoader.java:209) at javafx.fxml.FXMLLoader$ValueElement.processStartElement(FXMLLoader.java:592) at javafx.fxml.FXMLLoader.processStartElement(FXMLLoader.java:2366) at javafx.fxml.FXMLLoader.load(FXMLLoader.java:2177) at javafx.fxml.FXMLLoader.load(FXMLLoader.java:2076) now, calling loader.setController(null) between the load() calls is an obvious fix for this. However, if I do this in my real application, I get the strange behavior that all generated Nodes by the FXMLLoader refer to the same Controller. This picture shows our application how it should look like: http://250kb.de/ZCwoopb The nodes are generated using FXML and filled with data that is administrated by a controller class. Using setTemplate(true), the application throws the above error. Using setController(null) between the load() calls, this is what we get: http://250kb.de/yNu5dPV Notice that every single node is filled with the same content. Also note that the displayed content is the last loaded content. Please advice on how to use controllers when setTemplate(true) is set.
30-01-2013
Test fixed by adding warmup before actual measurement is done.
04-12-2012
Actually the feature is ok but the test is not correct. It needs warmup first after that a loading with different template flags can be compared.
04-12-2012
Test RT-23413test.java is failing.
04-12-2012
The "template" flag has been added and is currently awaiting API approval.
30-07-2012
I'd be totally fine with that.
26-07-2012
Here's what I'm currently thinking. We could add an isTemplate() method to FXMLLoader, along with setTemplate(boolean). When this flag is set to true, FXMLLoader#load() will: a) clear the root value by calling setRoot(null) b) not clear any existing imports When false (the default), the FXMLLoader will behave as currently implemented. Semantically, I think the name "template" makes sense, since what you are doing is using the content as a template for producing multiple instances of an object hierarchy. I also think it is easier to understand than "setClearImportsOnLoad()" plus calling setRoot(null) before each call to load(). So, in your case, you'd simply create a new FXMLLoader for a given resource URL, call setTemplate(true) on it, and then call load() every time you want to instantiate a new copy. What do you think?
25-07-2012
Yes. Using your example.fxml (which contains many more import PIs than my test case), I was able to reproduce the difference. Further, retaining the import cache between load() calls produces the same results as your custom class loader, so I think we're on the right track. However, I'm not entirely happy with the proposed API. It requires a caller to both set a flag indicating that the cache should not be cleared and set the document root to null before each call to load(), which is not very intuitive. I'll think about it a bit more and see if I can come up with a better approach. Let me know if you have any suggestions.
25-07-2012
could you reproduce the performance difference i described above?
25-07-2012
My bad - the reason CachingClassLoader executed faster was because the classes had already been loaded by the standard class loader. When I only run the CachingClassLoader test, the load time is the same as the standard class loader.
25-07-2012
>I suspect that something else is going on here, because I don't see anywhere near that kind of improvement. Can you post the test case that produces these actual numbers so I see how it performs on my system? These numbers come from a real-life application, therefore the numbers for the testcase I created that shows the issue are a bit different. // ============ example.fxml <?xml version="1.0" encoding="utf-8"?> <?import java.lang.?> <?import java.net.?> <?import javafx.scene.shape.?> <?import javafx.scene.paint.?> <?import javafx.scene.image.?> <?import javafx.geometry.?> <?import javafx.scene.text.?> <?import javafx.scene.web.?> <?import javafx.scene.?> <?import javafx.scene.control.?> <?import javafx.scene.layout.*?> <VBox xmlns:fx="http://javafx.com/fxml"> <Button text="button"/> <Label text="label"/> </VBox> // ========== MyClassLoader import java.io.IOException; import java.net.URL; import java.util.Enumeration; import java.util.HashMap; import java.util.Map; public class MyClassLoader extends ClassLoader{ private final Map<String, Class> classes = new HashMap<String, Class>(); private final ClassLoader parent; public MyClassLoader(ClassLoader parent) { this.parent = parent; } @Override public Class<?> loadClass(String name) throws ClassNotFoundException { Class<?> c = findClass(name); if ( c == null ) { throw new ClassNotFoundException( name ); } return c; } @Override protected Class<?> findClass( String className ) throws ClassNotFoundException { // System.out.print("try to load " + className); if (classes.containsKey(className)) { Class<?> result = classes.get(className); return result; } else { try { Class<?> result = parent.loadClass(className); // System.out.println(" -> success!"); classes.put(className, result); return result; } catch (ClassNotFoundException ignore) { // System.out.println(); classes.put(className, null); return null; } } } // ========= delegating methods ============= @Override public URL getResource( String name ) { return parent.getResource(name); } @Override public Enumeration<URL> getResources( String name ) throws IOException { return parent.getResources(name); } @Override public String toString() { return parent.toString(); } @Override public void setDefaultAssertionStatus(boolean enabled) { parent.setDefaultAssertionStatus(enabled); } @Override public void setPackageAssertionStatus(String packageName, boolean enabled) { parent.setPackageAssertionStatus(packageName, enabled); } @Override public void setClassAssertionStatus(String className, boolean enabled) { parent.setClassAssertionStatus(className, enabled); } @Override public void clearAssertionStatus() { parent.clearAssertionStatus(); } } //============ Main class import javafx.application.Application; import javafx.fxml.FXMLLoader; import javafx.scene.layout.VBox; import javafx.stage.Stage; import java.net.URL; public class Main extends Application{ public static ClassLoader cachingClassLoader = new MyClassLoader(FXMLLoader.getDefaultClassLoader()); public static void main(String[] args) { launch(args); } @Override public void start(Stage primaryStage) throws Exception { URL resource = getClass().getResource("example.fxml"); VBox root; FXMLLoader loader; long current = System.currentTimeMillis(); for (int i = 0; i<200; i++){ loader = new FXMLLoader(resource); root = (VBox) loader.load(); } long end = System.currentTimeMillis(); long result = end - current; System.out.println("duration for 200 without caching: "+ result +" ms, average: "+(result/200)); current = System.currentTimeMillis(); for (int i = 0; i<200; i++){ loader = new FXMLLoader(resource); loader.setClassLoader(cachingClassLoader); root = (VBox) loader.load(); } end = System.currentTimeMillis(); result = end - current; System.out.println("duration for 200 with caching: "+ result +" ms, average: "+(result/200)); } } // ================ my Results: duration for 200 without caching: 23057 ms, average: 115 duration for 200 with caching: 520 ms, average: 2
25-07-2012
I'm not sure what's going on yet, but there definitely seems to be some performance issue with the default implementation of ClassLoader#loadClass(). Just replacing the default class loader with this one cut the load time nearly in half (from ~6s to ~3s): public class CachingClassLoader extends ClassLoader { public CachingClassLoader(ClassLoader parent) { super(parent); } @Override public Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException { Class<?> type = findLoadedClass(name); if (type == null) { ClassLoader parent = getParent(); type = parent.loadClass(name); if (type == null) { type = findClass(name); } } if (type != null && resolve) { resolveClass(type); } return type; } } What is strange is that this is a literal translation of the steps described in the Javadoc for ClassLoader#loadClass(). So I'm not sure why the default implementation would be so much slower. I'll keep digging.
25-07-2012
> The crucial part about the performance are the misses of the class loads. Not exactly. A miss only happens when we don't first get a hit. And once the cache has been loaded, we don't get any more misses. > To find a VBox, the FXMLLoader does upon load() the following: > try to load java.lang.VBox, miss ... No - what actually happens is this (when we don't clear the import cache between loads, or when there are multiple occurrences of a given type within the same document): First load: - Look up "VBox" in import cache; not found - try to load java.lang.VBox, miss - try to load java.scene.control.VBox, miss - try to load java.scene.layout.VBox, found; add to cache and return java.scene.layout.VBox Second load: - Look up "VBox" in import cache; found, return java.scene.layout.VBox However, even with this optimization, performance did not appear to be impacted on subsequent loads (the performance was the same regardless of whether or not the import cache was cleared). > the start-up for 50 fxml files dropped from 60 seconds (!) to 250 ms) I suspect that something else is going on here, because I don't see anywhere near that kind of improvement. Can you post the test case that produces these actual numbers so I see how it performs on my system?
24-07-2012
The crucial part about the performance are the misses of the class loads. Everytime the ClassLoader tries to load a class that is not there it takes several ms. That there is no noticable impact on the performance is quite self-explanatory, since the found classes are cached in the classloader out-of-the-box. I.e.: consider following import statements at the start of the fxml file: <?import java.lang.?> <?import javafx.scene.control.?> <?import javafx.scene.layout.?> To find a VBox, the FXMLLoader does upon load() the following: try to load java.lang.VBox, miss try to load java.scene.control.VBox, miss try to load java.scene.layout.VBox, success on the next load() call, the FXMLLoader does this: try to load java.lang.VBox, miss try to load java.scene.control.VBox, miss try to load java.scene.layout.VBox, load from cache I'm pretty sure that this is performed since you can catch the calls in a custom class loader. The FXMLLoader should not only cache the found* classes but also the misses. And this has a massive impact on the performance (in my application, the start-up for 50 fxml files dropped from 60 seconds (!) to 250 ms).
24-07-2012
> Did you also cache the import calls he did not found? No. As noted above, FXMLLoader was modified such that the import cache was not cleared on every call to load. This means that, after the initial load (which populates the cache), every lookup should have resulted in a cache hit. However, this optimization did not appear to have a noticeable impact on performance.
24-07-2012
Did you also cache the import calls he did not found? So that loadClass or forName are only called once for java.lang.VBox for example?
24-07-2012
Based on some initial testing, I did not see a significant improvement when preserving import cache state across load() calls. I have reverted the changes related to this issue pending further investigation.
23-07-2012
This is resolved pending API approval. FXMLLoader no longer clears the imports when load() is called; instead, any previous imports are preserved. Callers can explicitly clear the imports via a new method named clearImports(); however, in practice, this won't generally be necessary, since FXMLLoader instances are typically only used once and then disposed.
19-07-2012