Support running server and client in the same process

[chapulina]

The GUI currently relies on the SceneBroadcaster system relaying the ECM state to stay in sync with the server. This operation is very costly, in fact, state broadcasting is one of the main performance bottlenecks when running the simulation with a GUI.

One possible workaround would be to run both client and server on the same process. Moreover, they could share the same ECM, so that there's no need to gather the entire state, serialize it, then update it on another process.

Some things to be careful about when looking into this approach:

• The Ogre rendering engines have a singleton which prevents instantiating 2 engines in the same process. So we'll need to either create 2 scenes in the same engine, or even better, use the same scene for sensor data generation and GUI.
• GuiSystems should continue not writing to the ECM, and instead perform requests through the UserCommands system, so that commands can be queued up for when we support undo / redo.
• We should still support running server and client separately. What will be the default in future releases will depend on performance and unforeseen caveats.

[diegoferigo]

For applications that integrate Ignition Gazebo as a library, this feature would be life-changing. Currently, the only way to integrate both server and gui is running the server in the main process, and programmatically spawn a new process with the gui (example). There are C++ libraries that help handling the life of external processes, however this is just a suboptimal workaround.

My main comment on a single server+gui process is about Qt. I'm not an expert GUI developer, but I remember past experiments failing due to the need of the Qt application to run in the main thread. In fact, from here I read:

Note that QCoreApplication::exec() must always be called from the main thread (the thread that executes main()), not from a QThread. In GUI applications, the main thread is also called the GUI thread because it's the only thread that is allowed to perform GUI-related operations.

[ahcorde]

I have started to take a look to this issue.

Right now we are using Ruby to launch the server and the GUI, in particular we are forking the threads in Ruby, I believe this is making more complex to share the ECM between them? Does it make sense to work first in this PR #694 ? which is adding standalone (C++) executables to ignition.

@mjcarroll and @chapulina thoughts?

[ahcorde]

Just for the records there is a closed PR which tries in to implement this feature https:/ignitionrobotics/ign-gazebo/pull/417/files

[chapulina]

Does it make sense to work first in this PR #694 ?

I think that PR may have conflicts to be resolved with the solution to this issue, but I don't think that PR is blocking this.

I expect the single process implementation to add a new function to ign.cc which combines what runServer and runGui are doing at the moment.

The tricky part when it comes to the ECM will be finding a way to:

• Share the ECM between a SimulationRunner and a GuiRunner
• Update the GUI plugins at the end of SimulationRunner::UpdateSystems, after PostUpdate

It's also important to keep the current behaviour working, so keep that in mind. For example, I imagine the GuiRunner will use the SetState mechanism when running in 2 processes, but that should be disabled when running in a single process.

@ahcorde , I recommend you start working on top of Fortress so you're comfortable making breaking changes. Once you have an idea of what the architecture could look like, feel free to open a draft PR with a proposal for review. After we've iterated on the high-level implementation, then you can clean it up and add tests. If it turns out that the implementation is backportable, we can rebase and retarget the PR. What do you think?

[ahcorde]

I have started to work in this draft PR #793

• Use the same ECM in the server and the client
• Keep th current behaviour working
• I have some issues with the renderUtil class. This class creates in the Scene3D and in the Sensor class a Scene which makes imposible to share the same renderUtil between them.
  • When we are running in different processes there is no problem because each process will create each own scene called "scene" inside each process. But when we are running this in the same process there is no guarantee when the RenderUtil::Init method is called, this will generate a race condition.
    • FIX: For now I have create a static mutex to handle the race condition.
  • Entities are added several times to the ECM, this generates some error messages
  • I'm not really sure which one (Scene3D or Sensor) should take the control of the RenderUtil
    • Should I create a different scenes ? which is the best way to handle this?

@chapulina or @iche033 thoughts ?

[ahcorde]

• Each one of the renderUtils has each own SceneManager, this manager is not shared between the Scene3D and the Sensor class.
  • Ogre will crash when we try to add the same object in both sceneManager
  • If we avoid to replicate entities in both sceneManager then the other sceneManager is not aware of the entities of the other manager.
  • Posible fix: Handle Ogre exception ?

[chapulina]

Should I create a different scenes ?

The ideal case would be to use the same scene, that should be better for performance and memory because we don't need to keep both of them updated. I don't know how hard this will be, but I think we should also have a single RenderUtil and SceneManager.

As discussed offline, I think this rewrite could be a good opportunity to look into how we could integrate ignition::gui::Scene3D into ign-gazebo and deprecate ignition::gazebo::Scene3d (gazebosim/gz-gui#137).

Posible fix: Handle Ogre exception ?

That would be working around the real problem, which is that we shouldn't have two separate scene managers trying to add entities to the scene. The proper solution would be to have a single manager that's bridging the rendering scene and the ECM.

[iche033]

+1 on using the same Scene / RenderUtil.

Some notes:

• rendering functions calls must happen in the same thread
• when there is GUI, Qt may want to be the one creating the GL context and you'll need to tell ogre to use the current GL context. If there is no GUI, we shouldn't make this call so that ogre will create the GL context.

[darksylinc]

My main comment on a single server+gui process is about Qt. I'm not an expert GUI developer, but I remember past experiments failing due to the need of the Qt application to run in the main thread. In fact, from here I read:

Note that QCoreApplication::exec() must always be called from the main thread (the thread that executes main()), not from a QThread. In GUI applications, the main thread is also called the GUI thread because it's the only thread that is allowed to perform GUI-related operations.

This sounds wrong. There is nothing special about the main() thread (at least for Windows/Linux/macOS. Android and iOS are another deal). However I suspect the wording is to prevent pointless bugs from clueless users starting with Qt and mixing UI calls in multiple threads, which are really hard to diagnose because it works but everything's glitchy.

UI applications should be fine as long as:

1. There is one thread designated as "main", for the terms of UI. No other thread processes UI messages (message and input polling, rendering)
2. The Qt "main thread" is the first thread to create a QObject instance. So refrain from doing ANY Qt operation until the worker thread that will handle Qt is created
3. This thread executes QCoreApplication::exec
4. The thread was created via OS primitive API and not QThread
5. The main() thread doesn't return (since that will signal process termination for the OS)

Note that ensuring points 1 & 2 are hard to enforce. It is very easy to call a UI operation from the wrong thread by accident

Note that iOS & Android are different though.

[scpeters]

there is some relevant discussion in the draft PR adding standalone executables for the ign tools: #694 (comment)