Page 1 of 1

Add async threads

Posted: Mon Jul 09, 2018 12:15 pm
by flaviojs
Short version:

Add a permanent thread to Universe.
That thread will execute all std::function<void(void)> that are added to a FIFO queue (scheduled).


(edit)
Use asynchronous threads to load and process data.
When done, the async thread will add the UI updates to the GUI work queue.
The GUI thread (main thread) will execute the work queue code before rendering the next frame.

PR: https://github.com/freeorion/freeorion/pull/2219

----

Long version:

The UI freezes were bothering me in single player games.
But I was really shocked when I tried being an observer in a multiplayer game with 6 AIs... the UI was frozen more than half of the time!!
It was so bad that I was only able to explore existing fleets and systems when the AIs started taking more than a minute to finish their turns.
In short, UI freezing became a much bigger issue for me. :x

In https://github.com/freeorion/freeorion/issues/2188 I learned that calculations were being done in the UI thread.
I can see that the current Universe isn't friendly to multi thread access.
Looking at the CMakeLists.txt you are targetting C++11, so you already have std::function, std::bind, and lambdas.

This proposal is a starting point to multi-threading, and consists of isolating Universe updates to a single thread (not the UI thread).
One way to do this is adding a thread to Universe, that would wait for work to be scheduled in a std::list<std::function<void(void)>> and execute it in FIFO order.

For single function calls scheduling with std::bind would be enough:

Code: Select all

GetUniverse().Schedule(std::bind(&Function, argument));
or
GetUniverse().Schedule(std::bind(&Class::MemberFunction, pointer_to_class_instance, argument));
For more complex code it could schedule a lambda:

Code: Select all

// do stuff in original thread
GetUniverse().Schedule([this, data] () {
  // do stuff in Universe thread (lambdas can only introduce variables in C++14)
  GetUniverse().UpdateSomething(data);
  this->m_atomic_var = GetUniverse().GetSomething();
  this->RequirePreRender();
});
// do stuff in original thread (probably before or during the lambda)
For reads, it might require a write lock but there are too many entry points, so I will think about it latter.
I'd like some feedback on this idea first. :)
What are your thoughts on this proposal?


(edit)
I tried adding a thread to Universe but it didn't seem appropriate. (post)
After trying a bunch of ways to split GUI code and data code, I submitted the most KISS alternative as a PR.

I think this approach is flexible and powerful, but there's still something I'm unsure about.
What should be done with unexpected exceptions in async threads?

Re: Add a thread to Universe

Posted: Mon Jul 09, 2018 1:20 pm
by Geoff the Medio
You would probably need to consider modifications the whole gamestate, not just the Universe and its contents.

Re: Add a thread to Universe

Posted: Mon Jul 09, 2018 8:21 pm
by flaviojs
I thought Universe and the Managers in the universe folder were the game state, and that the rest of the code accessed and manipulates these.
Is there more?

Re: Add a thread to Universe

Posted: Mon Jul 09, 2018 9:42 pm
by Geoff the Medio
Empires. Possibly supply, though it's debatable as I think it's all derived from other state and not independently meaningful. Possibly diplomacy.

Re: Add a thread to Universe

Posted: Mon Jul 09, 2018 11:06 pm
by flaviojs
Ok. It seems you're not against the idea of an asynchronous UI, so for now I'll give it a try and see it if it's feasible and appropriate.

Re: Add a thread to Universe

Posted: Tue Jul 10, 2018 7:04 pm
by o01eg
Will you consider option when your client asynchronously initiates scheduled process on server and then disconnects without stopping the game?

Re: Add a thread to Universe

Posted: Tue Jul 10, 2018 10:03 pm
by flaviojs
I'm not sure what you mean by that, but if it's handled correctly now then it will probably be handled correctly with a Universe thread too.
(I intend to keep equivalent functionality)

Re: Add a thread to Universe

Posted: Wed Jul 11, 2018 6:41 pm
by flaviojs
I see that changes would propagate all the way to the AI client and server.
However, freezing is essentially a GUI thing and they don't have a GUI, so a thread in Universe doesn't seem appropriate.

Next I'll try isolating GUI to a separate thread or adding a worker thread to the GUI (whatever needs less changes).

Re: Add a thread to Universe

Posted: Wed Jul 11, 2018 7:10 pm
by Dilvish
flaviojs wrote:However, freezing is essentially a GUI thing and they don't have a GUI, so a thread in Universe doesn't seem appropriate.
The server and AI pretty much actually need to just wait on Universe changes to be processed, before they can usefully proceed to a new task, right. I was figuring that you'd let the thread be an optional instantiation argument-- the server and AIs would leave it null and everything for them would essentially work the way it does now; for the human client it would instantiate the universe with a thread, which could then be used as you are envisioning. But it very well might be best to just make the thread part of the GUI as you are contemplating now.

ClientUI

Posted: Fri Jul 13, 2018 3:06 am
by flaviojs
I was trying to figure out where I should place read/write locking and other central thread-related facilities and got confused with ClientUI...

It looks like ClientUI wanted to be a singleton but is not quite there (the ClientUI code does not guarantee 0-1 instances).
HumanClientApp uses his own HumanClientApp::GetClientUI() and ClientUI::GetClientUI().

What is supposed to be the relationship between HumanClientApp (GG::SDLGUI), and ClientUI?

Re: Add async threads

Posted: Tue Jul 17, 2018 5:27 am
by flaviojs
I submitted the most KISS alternative as a PR (https://github.com/freeorion/freeorion/pull/2219) and updated the initial post.

I think this approach is flexible and powerful, but there's still something I'm unsure about.
What should be done with unexpected exceptions in async threads?

Re: Add async threads

Posted: Thu Jul 19, 2018 2:12 pm
by Geoff the Medio
flaviojs wrote:What should be done with unexpected exceptions in async threads?
Probably log the error and attempt to continue.

Re: Add async threads

Posted: Thu Jul 19, 2018 11:21 pm
by flaviojs
Geoff the Medio wrote:Probably log the error and attempt to continue.
Then the current implementation is fine. It logs an error and the program continues, only the async thread terminates.

----

Since I don't know if the explanation should be here, I'll copy the explanation on how this works from the PR:
Currently the main thread is used as GUI thread and processing thread, causing visible freezes every time processing takes a bit longer than an instant (for me it drops to 0/1 FPS quite often).

In OpenGL, almost every interaction must happen in the thread that created the OpenGL context. It is possible to create additional contexts but that would only complicate things.

If I want to separate processing from the GUI thread, then the processing code must have some way to tell the GUI thread that it can execute the UI code that depends on the processed data.

In this approach the processing code can add work (arbitrary code as a std::function<void()>) to a queue.
ClientUI is used as a singleton and is accessible to all UI code of the human client, so it is a suitable place to hold the FIFO work queue for the GUI thread.
HumanClientApp::HandleSystemEvents is called by the GUI thread just before rendering a frame, so it is a suitable place to execute the work that was added to the queue.

In short, GUI work is arbitrary code to be executed before rendering a frame, and any thread can add GUI work. Some renaming might be needed to make it clearer, but I can't think of better names...

----

Now then, let's explain the split in WaitingForGameStart...

When the GameStart message is received, HumanClientApp::HandleSystemEvents will call HumanClientApp::HandleMessage, which calls m_fsm->process_event(GameStart(msg));, which in turn calls WaitingForGameStart::react(const GameStart& msg).

The code here modifies Universe-related data and then updates the UI.

What I did was split that into two functions, WaitingForGameStart::ProcessData and WaitingForGameStart::ProcessUI, and make the first one execute in an async thread.

This is the new sequence:

1. GUI thread executes WaitingForGameStart::react(const GameStart& msg) and launches an async
2. async thread executes WaitingForGameStart::ProcessData while the GUI thread continues executing normally (processing keyboard events, processing mouse events, processing network messages, and rendering frames)
3. async is done and tells the GUI thread to execute WaitingForGameStart::ProcessUI (by pushing work to the queue)
4. GUI thread pops work from the queue and executes WaitingForGameStart::ProcessUI (before rendering the frame)

WaitingForGameStart::ProcessUI is called from HumanClientApp::HandleSystemEvents so it can't transit state (FSM isn't being processed), and since the FSM is only processed when a network message arrives I can't just post a DoneLoading event either (would be processed when a network message arrives).

Ideally the FSM would run in it's own thread, allowing you to post events from anywhere, but that would require splitting all UI code from the react functions of the FSM and that is not what this PR is about.

Since WaitingForGameStart::ProcessUI is already executing in the thread that processes the FSM, it is sufficient to call process_event(DoneLoading()) on the FSM.

Re: Add async threads

Posted: Sat Jul 21, 2018 12:38 am
by flaviojs
Maybe it would be clearer if I package the pattern into a class?

Code: Select all

enum ProcessingTaskStatus {
    CREATED,
    RUNNING_IN_ASYNC_THREAD,
    SCHEDULED_FOR_GUI_THREAD, //< only when the task updates the UI
    RUNNING_IN_GUI_THREAD, //< only when the task updates the UI
    DONE
};

class ProcessingTask : public std::enable_shared_from_this<ProcessingTask> {
public:
    ProcessingTask(bool updates_ui = true);
    virtual ~ProcessingTask();

    const ProcessingTaskStatus& Status() const;

    void Start(); // launches the async thread

    void operator()(); // public entry point for RunInAsync and RunInGUI (does the transition from async to GUI)
protected:
    virtual void RunInAsync() = 0; // executed in async thread
    virtual void RunInGUI(); // executed in GUI thread (only when the task updates the UI)
private:
    ProcessingTaskStatus m_status;
}
WaitingForGameStart::ProcessData would be <class derived from ProcessingTask>::RunInAsync
WaitingForGameStart::ProcessUI would be <class derived from ProcessingTask>::RunInGUI