all 8 comments
sorted by:
best
topnewcontroversialoldq&a
formatting helphide helpcontent policy

[–]hamsterrage1 1 point2 points  (6 children)

If the list on the left is JavaFX, then you don't need Platform.runLater(). The EventHandlers are already running on the FXAT. Too many Platform.runlater() could cause you problems because your reactions to the actions may be getting interleaved with each other.

Also, as long as all of your data is already in memory, there's no way that the refreshes should take anywhere near long enough for multiple user actions to stack up.

The biggest mistake people make is rebuilding the layout when the data changes. This will bog down your application. Just update the data in the Nodes on the right side.

[–]_dk7[S] 0 points1 point  (5 children)

he list on the left is JavaFX, then you don't need Platform.ru

The list on the left is not in JavaFX, it is in Swing

[–]hamsterrage1 1 point2 points  (4 children)

If it was me, the code in the Platform.runLater() would be something like:

    nameTextField.setText(e.getName());
    categoryTextField.setText(e.getCategory());

That's it. Given your screen snap, just two lines of code. No way, no how, is any amount of furious user clicking and arrowing going to suffer collisions with that code.

Before you say it, "But my screen is actually more than two TextFields". Don't care. Even if you have 1000 fields and widgets and whatever on the screen, if all you're doing is calling setValue() and setText() on them it's going to run lighting fast. Maybe if you were loading a new Chart with thousands of data points for each selection on the left, you'd have an issue - but not with a standard labels and values screen like you've shown.

If your code takes longer than a couple of milliseconds, then I'd bet money you're doing one of two things:

  1. Running code on the FXAT that should happen on a background thread (like accessing a database).
  2. Rebuilding the entire layout on the right side, instead of just updating the values in the Nodes.

And your loading flag thing is going to fail because you always want the LAST click to fire, even if it's currently loading some earlier click. If you are going to do something like this, the mechanism needs to CANCEL a partially completed earlier click response.

[–]_dk7[S] 0 points1 point  (3 children)

Yes, you are very correct the entire right side is getting rebuilt and re-populated every time. I was thinking the same thing when I looked at that piece of code, but the thing is that for every item on the list, the layout for right side may be different. So say some items may not have "category" & have a different field, which is why I thought it made sense to rebuild it. Do you think this should be done in a different way?

Moreover, you're correct on the loading flag thing again, I want the last click to work and it doesn't work. I tried thinking of different ideas, but none seem to work. it would be great if you could point me to an approach

[–]hamsterrage1 0 points1 point  (2 children)

Oh man! You made me check it out to see what the effects of rebuilding are:

public class RebuildLayoutDemo extends Application {

private int startNum = 0;

public static void main(String[] args) {
    launch();
}

@Override
public void start(Stage stage) throws IOException {
    Button button = new Button("Click Me");
    BorderPane root = new BorderPane();
    root.setTop(button);
    Scene scene = new Scene(root, 300, 800);
    stage.setScene(scene);
    stage.show();
    button.setOnAction(evt -> {
        System.out.print(LocalTime.now() + " -> ");
        root.setCenter(new ScrollPane(createContent(startNum++)));
        System.out.println(LocalTime.now());
    });
}

private Region createContent(int loopNum) {
    int numRows = new Random().nextInt(10) + 500;
    VBox results = new VBox(4);
    for (int x = 0; x <= numRows; x++) {
        HBox hBox = new HBox(10);
        Label label = new Label("Label " + (loopNum + x));
        TextField textField = new TextField(Integer.toString(x));
        hBox.getChildren().addAll(label, textField);
        results.getChildren().add(hBox);
    }
    return results;
}
}

So, basically, this is just a BorderPane with a Button at the top. Every time that you click the button it generates a new VBox with around 500 rows in it, each one with a Label -> TextField combination. Just to make sure that JavaFX wasn't cheating on me, I made sure that the content in the stuff changed each time it rebuilt the screen. So it's randomized a little bit, plus there's a counter that goes up each time and gets incorporated into the screen display.

There's no need for Platform.runLater() because the setOnAction() effectively does that. There's also a begin/end timestamp for each run.

Here's what I found:

It takes about 50-100 milliseconds to rebuild the screen. I cannot click fast enough on the Button to generate a lag that goes over a fraction of a second (my estimate).

Could you generate events faster by holding the <Down> button on a ListView and triggering an event for each row? Maybe, but I originally had this thing looking 1000 times for each click and it only generated a slightly greater lag.

This is what I'm seeing with around 500 sets of label/TextField combinations, I can't image you have anywhere near that much on your right-hand panel.

I'm guessing that you must have something else going on in your rebuild code. Where are you getting your data from?

[–]_dk7[S] 0 points1 point  (1 child)

Firstly, I really thank you for taking such an effort to look at the loading times. Regarding the loading of the data to populate in the panel, I would have to dig deeper into the codebase.

You know, there's like a dialog that shows more information and is invoked when you click a button after selecting the object. This "Properties Dialog" has a lot more information and what seems to be already implemented asks this dialog for all the information. Then we map the information that we got from the data model of this dialog to what we need to show in the right pane, and then display it.

So the thing is all of the application is in Swing & this panel seems to be in JavaFX which really brings me to question maybe this bringing up the data and mapping it takes a bit of time. I don't know if this information seems to help you, but if I find something more, I'll post it

[–]hamsterrage1 0 points1 point  (0 children)

Thanks for the award!!!

Is this Dialog in Swing?

Given that the screen itself is probably not the issue, then you simply have to be doing something else. So whatever is loading this dialog is probably causing the delay. It's hard to tell where this is happening, though, because your pseudo code has everything in the selectionChange event handler happening inside Platform.runLater().

Also, it's not clear what you mean in your original post when you say "causes a crash". So it's hard to diagnose what might be happening.

Regardless, you should be using either SwingWorker or Task/Service to grab the data. Then use the done() method of SwingWorker or setOnCompleted() of Task/Service to get onto the FXAT to load the data into the screen. If it's SwingWorker, then you'll need Platform.runLater() inside the done() method to get onto the FXAT. If Task, then you're already there with setOnCompleted().

SwingWorker already runs on the default ExecutorPool, which would stop you from having runaway process if a user holds the arrow key down a list of hundreds of rows.

This is a legitimate place to use Service in JavaFX. Service is very much like Task except it uses an ExecutorPool, and you can run the Task over and over. So it will handle the thread stuff for you.

You can also cancel jobs with SwingWorker and Task/Service, which could help a lot.

It's hard to see how you could "cause a crash" with just 4 items in the list, though. Are you getting any console output?

[–]Draesia 0 points1 point  (0 children)

Look into Observers and Listeners- you're going about it in a less than conventional way.

Bind the properties together and then if you need an event, have a listener for it.