Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load FXML in background thread with FluentViewLoader #451

Closed
rguillens opened this issue Nov 16, 2016 · 9 comments
Closed

Load FXML in background thread with FluentViewLoader #451

rguillens opened this issue Nov 16, 2016 · 9 comments

Comments

@rguillens
Copy link
Contributor

Could be possible loading large FXML scenegraphs in background thread and then execute initialization code in UI thread?

@manuel-mauky
Copy link
Collaborator

This sounds interesting and could improve performance of loading a lot. However, I don't think it's possible with the current implementation.
We are using the original FXMLLoader internally and this class doesn't allow much configuration.

To implement this we would need to first check if it is even possible to load FXML files from another thread with FXMLLoader. I don't think this is possible because typically you need to be on the FX-Thread to create instances of JavaFX controls.

If I'm wrong and loading FXML files from outside of FX thread is possible the second step would be to introduce a new initialization mechanism that is guaranteed to be executed on the FX thread. This would be the easy part. For example we could introduce a new interface that we check at loading time and invoke the new initialize method. alternatively you could use Platform.runLater in the normal initialize method.

@rguillens
Copy link
Contributor Author

rguillens commented Nov 17, 2016

I think it is possible to load FXML within a Task, here is an example in SO:

http://stackoverflow.com/questions/34873673/load-fxml-as-background-process-javafx

I modified the helloworld mini-example to load the FXML file using FluentViewLoader in a JavaFx Task according to your recommendation of using Platform.runLater in the normal initialize method and it worked fine.
These are the most important changes:

  • In the application starter class
public class Starter extends Application {

...

    @Override
    public void start(Stage stage) throws Exception {
        stage.setTitle("Hello World Application");

        LoadFXMLTask loadFXMLTask = new LoadFXMLTask();

        loadFXMLTask.setOnSucceeded(event -> {
            final Parent root = (Parent) event.getSource().getValue();
            stage.setScene(new Scene(root));
            stage.show();
        });

        Thread thread = new Thread(loadFXMLTask);
        thread.start();
    }

    class LoadFXMLTask extends Task<Parent> {

        @Override
        protected Parent call() throws Exception {
            final ViewTuple<HelloWorldView, HelloWorldViewModel> viewTuple = 
                    FluentViewLoader.fxmlView(HelloWorldView.class).load();
            return viewTuple.getView();
        }
    }
  • In the view
public class HelloWorldView implements FxmlView<HelloWorldViewModel>, Initializable {

...

    @Override
    public void initialize(URL url, ResourceBundle resourceBundle) {
        Platform.runLater(() -> helloLabel.textProperty().bind(viewModel.helloMessage()));
    }

@manuel-mauky
Copy link
Collaborator

Thanks for the example code. It looks interesting. Do you see a performance win with this approach?
I will try this out within the next days with one of our bigger applications to see if there is any problem with the FX thread.

To support this directly in the framework I think of two aspects:

  • Add loadAsync() to FluentViewLoader:
CompletableFuture<ViewTuple<MyView, MyViewModel>> loadingResult = FluentViewLoader.fxmlView(MyView.class).loadAsync();

CompletableFuture<MyView> viewFuture = loadingResult.thenApply(ViewTuple::getView);

Parent root = viewFuture.get();

Personally, instead of Task I would use CompletableFuture as result type because of the possibility of monadic composition but we can discuss this.

  • Add InitializeOnFxThread Interface:
public class MyView implements FxmlView, InitializeOnFxThread {

    public void initialize() {
        // don't use this.
    }

    @Override
    public void initializeOnFxThread() {
         // use this on FX thread
    }
}

In the ViewLoader we can make sure that this is invoked on the FX Thread by wrapping it internally in a Platform.runLater. It would be even cooler if we could enforce that the normal initialize method is always invoked on the FX thread but I don't see any way of doing this.

However, While I have an idea of how this could look in the code I'm still not sure if we should add this to the library. Especially the Initialize Interface isn't obvious and it's not clean that the components itself are depending on how the ui is loaded. Maybe we should only document the solution that you described here?

@rguillens
Copy link
Contributor Author

rguillens commented Nov 18, 2016

Thank you very much for your time, the solution you propose is very elegant. However in this case I prefer to use JavaFx Tasks over CompletableFuture, because they provide the possibility of monitoring the execution of the task, in this case the loading of FXML files, as well as creating a more friendly experience for the end user. I will formulate a more elaborate solution proposal using JavaFx Tasks that we can discuss later.
As for initialization I agree with you, if the FXMLLoader class is not extensible at this point, to ensure that it occurs within the FX Thread, then it is not worth creating the new InitializeOnFxThreadinterface.
If so, I do not think it is right to induce fragmentation or duplication of the initialization code of the components according to the way the UI loads. I do not think it is viable to add this functionality in this way to the library because in many cases it would be problematic.
Regards.

@bekwam
Copy link
Contributor

bekwam commented Nov 19, 2016

Manuel, Have you considered the InputStream version of the FXMLLoader.load() call? Maybe there's a speedup if IO is done in a separate thread producing sets of bytes for the FluentViewLoader?

@manuel-mauky
Copy link
Collaborator

Hi carl.
I don't think that this would bring a noticeable speedup because it would only affect the root fxml file. All other embedded files are handled internally by the fxmlloader.
However, of course we can try it out and maybe I'm wrong.

In general I don't think that putting the loading in a separate thread will bring us a real performance win because then the whole loading process is still done in one big step. It just on another thread so the ui isn't blocked.
To get a real speedup the fxmlloader itself would need to load parts of the fxml tree in parallel on several threads. We even had thought about creating our own fxmlloader in the past (for other reasons) but this would be a huge amount of work.

@bekwam
Copy link
Contributor

bekwam commented Nov 19, 2016

No, you're right. I ran a few test programs that loaded the FXML into byte arrays using various numbers of Threads. IO isn't the performance bottleneck.

@manuel-mauky
Copy link
Collaborator

Ok, to sum up the discussion:

  1. We won't introduce an API for async loading at the moment because most likely there won't be much benefit in terms of loading performance.
  2. We will describe in the wiki how you can still make the loading asynchronous by using JavaFX Tasks or CompletableFuture with the goal of not blocking the UI thread.

@manuel-mauky
Copy link
Collaborator

Added a wiki page for this topic: https://github.com/sialcasa/mvvmFX/wiki/Asynchronous-Operations

I will close this issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants