Conversation
neverorfrog
requested review from
DaniAffCH,
EugenioBugli,
FlavioFoxes and
ValerioSpagnoli
|
GG. Ho lasciato solo un commento su una cosa perché non l'ho capita, però il resto mi pare ok |
DaniAffCH
left a comment
DaniAffCH
left a comment
There was a problem hiding this comment.
Ci sono cose che non mi sono chiarissime, soprattutto rispetto alla concorrenza. Per il resto va bene imo
| void updateSnapshot(); | ||
|
|
||
| // Get thread-safe read access to snapshot | ||
| mjData* getSnapshot() const { | ||
| return dataSnapshot; | ||
| } | ||
|
|
There was a problem hiding this comment.
Non capisco perchè la gestione degli snapshot è thread safe.
Mi sembra che tu fai lock sul write ma mai sul read. Quindi se qualcuno potrebbe cambiare il dato durante una read. No?
Inoltre l'unico che chiama updateSnapshot è SimulationThread::run(). Quindi in quali casi si ha una scrittura concorrente?
There was a problem hiding this comment.
stavo facendo best effort, ma effettivamente era sbagliato. ho messo il lock anche in lettura
| std::vector<std::future<void>> futures; | ||
|
|
||
| { | ||
| std::lock_guard lock(mutex_); | ||
| for (std::shared_ptr<Robot> r : robots_) { | ||
| futures.push_back(threadPool_.enqueue([r]() { r->update(); })); | ||
| } | ||
| } | ||
|
|
||
| // Wait for all robot updates to complete | ||
| for (auto& future : futures) { | ||
| future.wait(); |
There was a problem hiding this comment.
Perchè fare gli update in parallelo e joinare subito dopo è necessario?
There was a problem hiding this comment.
Perché altrimenti un robot potrebbe sovrascrivere lo snapshot acceduto da un altro
3 participants
This pull request introduces improvements to the simulation's multi-threaded rendering and sensor update infrastructure. The main changes are the addition of a thread pool for concurrent robot updates, a robust EGL-based offscreen rendering system for camera sensors, and improved thread safety and performance in sensor data handling.
Rendering and EGL Integration:
SharedEGLDisplay) and per-camera contexts (CameraContext). Cameras now lazily initialize their EGL context on worker threads and use MuJoCo's snapshot data for rendering (Mujoco best practice) [1] [2] [3] [4] [5]Threading and Concurrency:
ThreadPoolutility for parallelizing robot updates inRobotManager, enabling each robot's update to run concurrently and improving simulation performance. The pool size is set to the number of hardware threads. [1] [2] [3] [4]SimulationThreadto operate on the entireMujocoContextobject, facilitating thread-safe access to simulation and snapshot data. [1] [2]Sensor Architecture and Data Handling:
Sensorclass to support frequency-limited updates using time intervals, preventing unnecessary computation and improving determinism. [1] [2]MuJoCo Context Management:
MujocoContextnow maintains a separate snapshot of simulation data for safe concurrent reading by sensors, and provides thread-safe methods for updating and accessing this snapshot. [1] [2]