gz-sim icon indicating copy to clipboard operation
gz-sim copied to clipboard

Run server and client in the same process

Open ahcorde opened this issue 3 years ago • 14 comments

Signed-off-by: ahcorde [email protected]

🎉 New feature

Summary

Related to this issue https://github.com/ignitionrobotics/ign-gazebo/issues/556

Test it

ign gazebo -r -v 4 --same-process shapes.sdf
ign gazebo -r -v 4 --same-process camera_sensor.sdf  # -> crash

Checklist

  • [x] Signed all commits for DCO
  • [ ] Added tests
  • [ ] Added example and/or tutorial
  • [ ] Updated documentation (as needed)
  • [ ] Updated migration guide (as needed)
  • [ ] codecheck passed (See contributing)
  • [ ] All tests passed (See test coverage)
  • [ ] While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

ahcorde avatar Apr 27 '21 10:04 ahcorde

I made a comment in the related issue https://github.com/ignitionrobotics/ign-gazebo/issues/556#issuecomment-827510853

I want to make it work then I will clean up te code.

ahcorde avatar Apr 27 '21 10:04 ahcorde

I updated the base PR to match with the recent refactoring in the scene3D.

Current status

  • Gui and sensor thread shared the same scene.
    • not able to visualize the objects in the scene 3D
    • When I try to load the ImageDisplay then I get a crash
  • Ogre crashes when we try to use the wheel to zoom in and zoom out. In particular in the method OgreRayQuery::ClosestPoint. The vertex from the meshes are not fetched properly.
  • Ogre2 crashes on start in the method void Ogre2RenderTarget::Render() in the call engine->OgreRoot()->renderOneFrame();

to reproduce:

/home/ahcorde/ignition_fortress_leak/install/bin/ign gazebo --gui-config src/ign-gazebo/src/gui/gui.config camera_sensor.sdf -v 4 --same_process --render-engine ogre --render-engine-gui ogre --render-engine-server ogre

If you want to try Ogre two, you should modify the gui.config file and replace ogre with ogre2

/home/ahcorde/ignition_fortress_leak/install/bin/ign gazebo --gui-config src/ign-gazebo/src/gui/gui.config camera_sensor.sdf -v 4 --same_process --render-engine ogre2 --render-engine-gui ogre2 --render-engine-server ogre2

Limitations

everything should run with ogre or ogre2 but we cannot mix both renders when gui and server are running in the same process.

ahcorde avatar Jun 02 '21 15:06 ahcorde

I have two issues right now:

  • Sometimes ign-gazebo starts with some repeated entities in the Entity tree

Selección_102

  • When the entity is removed from the entity tree, it removed from the scene but not from the entity tree

same_process_remove_entity

ahcorde avatar Jun 04 '21 16:06 ahcorde

Decisions that I have made

There are two differences main way to launch ign-gazebo: with sensors and without sensors. This have some implications.

When we are runninig with sensors

There are two RenderUtils available: One in the sensor system plugin and another in the GUI. This is the easiest case because there are two system plugins that remove entities: Sensors and Physics which runs at the same speed. But the Scene 3D is running just at 30Hz. This may cause some issues in the Entity tree plugin. Some of the EachRemoved cycles are lost. I have created some events to be able to remove/add entities in the Entity tree plugin, this event are emitted in the renderutils class.

  • RemoveFromECM
  • AddToECM

The GzSceneManager should not update renderutil because the sensor thread will do it.

When running without sensors

In this case the physics system plugin will be the only one with access to the removeEach calls. In this case this plugin will send an event called UpdateGUIECM to avoid missing any removals.

There is another event called EnableSensors this event is used GzSceneManager. In this case the sensor system plugin is not updating the RenderUtils because it not running, so this event allow us to check if we should update the Scene 3D from the Gui system plugin.

  • EnableSensors
  • UpdateGUIECM

Render

On of the limitiation that we have, it is that we can only update the render scene in the same thread, otherwise we will get a crash. When running in the same process there are two Renderutils trying to update the scene3d in different threads, it's important to load in the first time the Scene3D to set the option useCurrentGLContext to 1. For this reason we need to create a event called Render, this event is emitted in the GzSceneManager plugin every time that an event Render is received in the eventFilter method.


Notes

  • Run the world without the sceneBroadcaster when using the --same_process flag
  • We can use Ogre or Ogre2 at the same time, we cann't mix both render engines when same process is activated.
  • Recommended branch of ign-gui https://github.com/ignitionrobotics/ign-gui/pull/new/ahcorde/6/new_scene

ahcorde avatar Jun 08 '21 10:06 ahcorde

Recommended branch of ign-gui https://github.com/ignitionrobotics/ign-gui/pull/new/ahcorde/6/new_scene

ahcorde avatar Jun 08 '21 10:06 ahcorde

With the latest changes, the events for keeping GUI plugins in sync with server side seems to be working from testing.

I also want to brainstorm ideas on how we could reuse more of the current ECM infrastructure. I think the problem all came down to the GuiRunner running the gui plugins at 30 Hz in a separate thread. What do you think of the following proposal:

  • Emit an event when SimulationRunner updates the systems https://github.com/ignitionrobotics/ign-gazebo/blob/cc4cf71f45836ad6478ab8c69af5dc4a19201d5d/src/SimulationRunner.cc#L523-L527
  • In GuiRunner same process mode, we can connect to this event instead of running its own loop.
  • In call back we can call UpdatePlugins. We can also throttle it to some rate, e.g. 30Hz. by checking time since last update call.
  • In order not to miss important events like entities removed / added, we can use ECM's HasNewEntities, HasEntitiesMarkedForRemoval, HasOneTimeComponentChanges functions in the callback and check if we need bypass throttling and call UpdatePlugins https://github.com/ignitionrobotics/ign-gazebo/blob/cc4cf71f45836ad6478ab8c69af5dc4a19201d5d/include/ignition/gazebo/EntityComponentManager.hh#L493-L502

The idea above is so that the gui plugins can keep using the EachRemoved call without having to connect to new events. This reduces the effort required to port other gui plugins (or user's own custom gui plugins) to work in the same process mode.

iche033 avatar Jun 09 '21 00:06 iche033

@iche033, your proposal it's much more simpler https://github.com/ignitionrobotics/ign-gazebo/pull/793/commits/5521b407e41376643d2064a79643f52673c73f4b

ahcorde avatar Jun 09 '21 08:06 ahcorde

I made a quick test with the 3k_shapes.world.

This is the data:

Two processes same process
mean 0.008285111 0.009956856
median 0.0083 0.01
min 0.0075 0.0097
max 0.0085 0.0101

20% of improvement

Orage line is with ignition running in the same process, blue in two processes:

Figure_1

ahcorde avatar Jun 11 '21 12:06 ahcorde

@chapulina do you mind to take a look ?

ahcorde avatar Jun 16 '21 13:06 ahcorde

I made a quick test with the 3k_shapes.world.

20% of improvement

Orage line is with ignition running in the same process, blue in two processes:

Figure_1

Is the y-axis RTF (so, for example, I assume .01 would be 1% RTF), or something else?

adlarkin avatar Jun 18 '21 11:06 adlarkin

@adlarkin Yes, you are right

ahcorde avatar Jun 18 '21 13:06 ahcorde

@chapulina what do you think about this new method in GuiSystem.hh ?

    public: virtual void Configure(
      EventManager &/*_event*/, bool /*sameProcess*/){}

ahcorde avatar Jun 22 '21 21:06 ahcorde

Codecov Report

Merging #793 (74ed8e2) into main (c992f31) will increase coverage by 0.60%. The diff coverage is 35.99%.

:exclamation: Current head 74ed8e2 differs from pull request most recent head 449e6e4. Consider uploading reports for the commit 449e6e4 to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##             main     #793      +/-   ##
==========================================
+ Coverage   63.40%   64.00%   +0.60%     
==========================================
  Files         240      247       +7     
  Lines       19548    20036     +488     
==========================================
+ Hits        12394    12825     +431     
- Misses       7154     7211      +57     
Impacted Files Coverage Δ
include/ignition/gazebo/Server.hh 100.00% <ø> (ø)
include/ignition/gazebo/ServerConfig.hh 100.00% <ø> (ø)
include/ignition/gazebo/rendering/RenderUtil.hh 100.00% <ø> (ø)
src/Server.cc 79.54% <0.00%> (-7.21%) :arrow_down:
src/SimulationRunner.hh 100.00% <ø> (ø)
src/gui/plugins/entity_tree/EntityTree.cc 12.97% <0.00%> (+0.13%) :arrow_up:
src/gui/plugins/scene3d/Scene3D.hh 50.00% <ø> (ø)
src/gui/Gui.cc 61.31% <23.07%> (-4.05%) :arrow_down:
src/gui/plugins/scene3d/Scene3D.cc 11.24% <26.66%> (+0.38%) :arrow_up:
src/rendering/SceneManager.cc 35.25% <27.58%> (+6.40%) :arrow_up:
... and 51 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c992f31...449e6e4. Read the comment docs.

codecov[bot] avatar Aug 10 '21 18:08 codecov[bot]

Didn't make it into beta, can come in a minor release.

chapulina avatar Sep 22 '21 15:09 chapulina