FreeOrion

Forums for the FreeOrion project
It is currently Mon Jul 23, 2018 5:24 pm

All times are UTC




Post new topic Reply to topic  [ 14 posts ] 
Author Message
 Post subject: Add async threads
PostPosted: Mon Jul 09, 2018 12:15 pm 
Offline
Krill Swarm

Joined: Sat Jul 07, 2018 7:24 pm
Posts: 12
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:
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:
// 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?


Last edited by flaviojs on Tue Jul 17, 2018 5:26 am, edited 1 time in total.

Top
 Profile  
 
PostPosted: Mon Jul 09, 2018 1:20 pm 
Offline
Programming, Design, Admin
User avatar

Joined: Wed Oct 08, 2003 1:33 am
Posts: 12237
Location: Munich
You would probably need to consider modifications the whole gamestate, not just the Universe and its contents.


Top
 Profile  
 
PostPosted: Mon Jul 09, 2018 8:21 pm 
Offline
Krill Swarm

Joined: Sat Jul 07, 2018 7:24 pm
Posts: 12
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?


Top
 Profile  
 
PostPosted: Mon Jul 09, 2018 9:42 pm 
Offline
Programming, Design, Admin
User avatar

Joined: Wed Oct 08, 2003 1:33 am
Posts: 12237
Location: Munich
Empires. Possibly supply, though it's debatable as I think it's all derived from other state and not independently meaningful. Possibly diplomacy.


Top
 Profile  
 
PostPosted: Mon Jul 09, 2018 11:06 pm 
Offline
Krill Swarm

Joined: Sat Jul 07, 2018 7:24 pm
Posts: 12
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.


Top
 Profile  
 
PostPosted: Tue Jul 10, 2018 7:04 pm 
Offline
Space Kraken

Joined: Sat Dec 10, 2011 5:46 am
Posts: 135
Will you consider option when your client asynchronously initiates scheduled process on server and then disconnects without stopping the game?

_________________
Gentoo Linux x64, gcc-7.3, boost-1.65.0
Ubuntu Server 18.04 x64, gcc-7.3, boost-1.65.1
Welcome to multiplayer server at freeorion-test.dedyn.io.Version 2018-07-13.9a06376 0.4.8 RC2
Donates are welcome: BTC:14XLekD9ifwqLtZX4iteepvbLQNYVG87zK


Top
 Profile  
 
PostPosted: Tue Jul 10, 2018 10:03 pm 
Offline
Krill Swarm

Joined: Sat Jul 07, 2018 7:24 pm
Posts: 12
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)


Top
 Profile  
 
PostPosted: Wed Jul 11, 2018 6:41 pm 
Offline
Krill Swarm

Joined: Sat Jul 07, 2018 7:24 pm
Posts: 12
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).


Last edited by flaviojs on Wed Jul 11, 2018 7:16 pm, edited 1 time in total.

Top
 Profile  
 
PostPosted: Wed Jul 11, 2018 7:10 pm 
Offline
AI Lead, Programmer
User avatar

Joined: Sat Sep 22, 2012 6:25 pm
Posts: 4683
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.

_________________
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0


Top
 Profile  
 
 Post subject: ClientUI
PostPosted: Fri Jul 13, 2018 3:06 am 
Offline
Krill Swarm

Joined: Sat Jul 07, 2018 7:24 pm
Posts: 12
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?


Top
 Profile  
 
 Post subject: Re: Add async threads
PostPosted: Tue Jul 17, 2018 5:27 am 
Offline
Krill Swarm

Joined: Sat Jul 07, 2018 7:24 pm
Posts: 12
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?


Top
 Profile  
 
 Post subject: Re: Add async threads
PostPosted: Thu Jul 19, 2018 2:12 pm 
Offline
Programming, Design, Admin
User avatar

Joined: Wed Oct 08, 2003 1:33 am
Posts: 12237
Location: Munich
flaviojs wrote:
What should be done with unexpected exceptions in async threads?
Probably log the error and attempt to continue.


Top
 Profile  
 
 Post subject: Re: Add async threads
PostPosted: Thu Jul 19, 2018 11:21 pm 
Offline
Krill Swarm

Joined: Sat Jul 07, 2018 7:24 pm
Posts: 12
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:
Quote:
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.


Top
 Profile  
 
 Post subject: Re: Add async threads
PostPosted: Sat Jul 21, 2018 12:38 am 
Offline
Krill Swarm

Joined: Sat Jul 07, 2018 7:24 pm
Posts: 12
Maybe it would be clearer if I package the pattern into a class?
Code:
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


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 14 posts ] 

All times are UTC


Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group