1

My app has background threads that need to access the UI. Imagine a chess program (AI) that "thinks" for a number of seconds before it plays a move on the board. While the thread runs the UI is blocked for input but there is still output.

There are 3 threads involved:

  1. the CN1 EDT
  2. the think thread, using invokeAndBlock, that outputs information about the search process (in a TextField), such as the current move, search depth and search value
  3. a clock thread, started with Thread.start(), that updates once per second the time used by White or Black (TextFields)

During the search (invokeAndBlock) the stopButton is accessible to force the search to stop (not shown).

Below is my current implementation. It works and my question is: is it the right way to implement this?

(I read https://www.codenameone.com/blog/callserially-the-edt-invokeandblock-part-1.html and part-2.)

Form mainForm;
TextField whiteTime, blackTime;     // updated by clock thread
TextField searchInfo;               // updated by think thread
Clock clock;
Move move;

public void start() {
    ...
    mainForm = new Form(...);
    ...
    thinkButton.addActionListener((ActionListener) (ActionEvent evt) -> {
        think();
    });
    mainForm.show();
}

void think() {
    blockUI();                      // disable buttons except stopButton
    clock.start(board.player);      // this thread calls showWhiteTime or showBlackTime every second
    invokeAndBlock(() -> {          // off the EDT
        move = search(board, time); // e.g. for 10 seconds
    });
    clock.stop();
    animateMove(board, move);
    clock.start(board.player);
    freeUI();
}

// search for a move to play
Move search(Board board, int time) {
    ...
    while (time > 0) {
        ...
        showSearchInfo(info);       // called say a few times per second
    }
    return move;
}

void showSearchInfo(String s) {     // access UI off the EDT
    callSerially(() -> {            // callSerially is necessary here
        searchInfo.setText(s);
    });
}

void showWhiteTime(String s) {
    whiteTime.setText(s);           // no callSerially needed, although off the EDT (?)
}

void showBlackTime(String s) {
    blackTime.setText(s);           // no callSerially needed, although off the EDT (?)
}

Edit: new versions of think, showWhiteTime and showBlackTime.

// version 2, replaced invokeAndBlock by Thread.start() and callSerially
void think() {
    blockUI();                      // disable buttons except stopButton
    new Thread(() -> {              // off the EDT
        clock.start(board.player);  // this thread calls showWhiteTime or showBlackTime every second
        move = search(board, time); // e.g. for 10 seconds
        clock.stop();
        callSerially(() -> {
            animateMove(board, move);
            clock.start(board.player);
            freeUI();
        });
    }).start();
}

// version 2, added callSerially
void showWhiteTime(String s) {      // access UI off the EDT
    callSerially(() -> {
        whiteTime.setText(s);
    });
}

// version 2, added callSerially
void showBlackTime(String s) {      // access UI off the EDT
    callSerially(() -> {
        blackTime.setText(s);
    });
}
J-J
  • 419
  • 2
  • 12
  • Re, "background threads that need to access the UI." Don't go there. Coupling between the UI thread and other threads should be very loose. UI thread should display the _state_ of the program. Other threads should have a means to notify the UI when they have changed that state in ways that the user might want to see. The UI should have means to notify the background threads that the user has changed the state in ways that might be important. But mostly, they should run independently of each other. – Solomon Slow Jun 23 '20 at 18:47
  • Re, "blockUI()...search()...freeUI()." Don't go there either. The UI should always be responsive to user actions even if the response is just to say, "You can't do that right now because it's not your turn to move." An unresponsive UI creates the illusion that your program is unreliable even when it isn't. A snappy UI can create the illusion that your program is faster than it really is. – Solomon Slow Jun 23 '20 at 18:51
  • "Don't go there" is not a useful answer. Have you ever used a chess program? – J-J Jun 23 '20 at 19:02
  • Wasn't intended to be an answer. Was intended to be an alternative design suggestion. Yes, I'm not a serious player, but I have played against a machine. And I would not recommend a chess program to any of my friends if its user interface locked up whenever it was "thinking." – Solomon Slow Jun 23 '20 at 19:36
  • Okay thanks, I will think about that. But the question is really about Codename One threads and accessing the UI. – J-J Jun 23 '20 at 19:47
  • @SolomonSlow Codename One supports a method of safely "blocking the UI" without actually "blocking it". That part of the code is fine – Shai Almog Jun 24 '20 at 02:03

1 Answers1

1

Most of the code is fine though I would avoid the EDT violations you have in showWhiteTime and showBlackTime. EDT violations can fail in odd ways all of a sudden since you trigger async operations and things can turn nasty quickly. I suggest turning on the EDT violation detection tool in the simulator.

Two things to keep in mind when using invokeAndBlock:

  • It's slower than a regular thread
  • It blocks pending events in some cases so it's problematic to have it as a part of a pending event chain

The second point is a difficult one to grasp and a source of many mistakes so it's worth explaining a bit.

consider this code:

 buttonA.addActionListener(e -> {
       doStuff();
       invokeAndBlock(...);
       doOtherStuff();
 });
 buttonA.addActionListener(e -> doSomethingImportant());

That might not seem realistic as you usually don't add two separate listeners one after the other but this happens enough e.g. if one change triggers another etc.

The current event processing will be blocked for buttonA during invokeAndBlock. That means that doOtherStuff() will wait for the invokeAndBlock and also doSomethingImportant() will wait.

If doSomethingImportant() shows another form you can end up with weird behavior such as ages after you pressed the button and did a lot of other things suddenly your form changes.

So you need to be very conscious of your usage of invokeAndBlock.

Shai Almog
  • 51,749
  • 5
  • 35
  • 65
  • Thank you for your answer. Yes I already use full EDT violation detection in the simulator. It reports a violation if I don't use callSerially in showSearchInfo. However, it does not report violations in showWhiteTime and showBlackTime. Do I understand correctly that you suggest to use callSerially in those methods as well? (See Edit in question.) – J-J Jun 24 '20 at 16:46
  • Do you also suggest to avoid invokeAndBlock in this case? How about version 2 of think()? (See Edit in question.) – J-J Jun 24 '20 at 16:47
  • Violation detection isn't perfect. It can miss some things that are problems and alert about things that aren't. Yes, you should use callSerially if you access the UI from a separate thread even if this isn't detected by the violation detection tool. The async approach would be faster but not as readable. It's your choice, both will work. – Shai Almog Jun 25 '20 at 02:27