Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up Resource Spawner load time by fetching model list asynchronously #1962

Merged
merged 5 commits into from
May 3, 2023

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Apr 11, 2023

🦟 Bug fix

Fixes #1253

Summary

The main reason that the Resource Spawner took a long time to load is that it tried to fetch the list of all available models on Fuel instead of just the selected owner. And it did that while blocking the Qt thread, so the user was unable to interact with the GUI while the model list was being fetched. The approach I've taken is to only fetch the list of models for the default owner ("openrobotics") and allow users to add/remove any owner they want. The fetching is also done in a separate thread so as to not block the GUI.

This currently does not preserve the list of owners added by the user. I thought it would be better to add that in a follow up PR at a future time.

Needs gazebosim/gz-fuel-tools#350

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • 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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

…usly

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey requested a review from mjcarroll as a code owner April 11, 2023 05:27
@azeey azeey self-assigned this Apr 11, 2023
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Apr 11, 2023
@osrf-triage osrf-triage added this to Inbox in Core development Apr 11, 2023
@azeey azeey added the needs upstream release Blocked by a release of an upstream library label Apr 11, 2023
@azeey azeey moved this from Inbox to In review in Core development Apr 17, 2023
@azeey azeey requested a review from nkoenig April 17, 2023 15:19
@mjcarroll
Copy link
Contributor

This one has conflicts now.

azeey added 2 commits May 2, 2023 12:45
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey removed the needs upstream release Blocked by a release of an upstream library label May 2, 2023
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great improvement. LGTM with green CI!

@codecov
Copy link

codecov bot commented May 3, 2023

Codecov Report

Merging #1962 (fa22f51) into ign-gazebo3 (a2a2c85) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

❗ Current head fa22f51 differs from pull request most recent head 606f209. Consider uploading reports for the commit 606f209 to get more accurate results

@@               Coverage Diff               @@
##           ign-gazebo3    #1962      +/-   ##
===============================================
- Coverage        78.04%   78.03%   -0.02%     
===============================================
  Files              255      255              
  Lines            15080    15082       +2     
===============================================
  Hits             11769    11769              
- Misses            3311     3313       +2     
Impacted Files Coverage Δ
src/gz.cc 63.88% <0.00%> (-0.72%) ⬇️

@azeey azeey merged commit 2a12514 into gazebosim:ign-gazebo3 May 3, 2023
Core development automation moved this from In review to Done May 3, 2023
@azeey azeey deleted the resource_spawner branch May 3, 2023 04:14
@azeey azeey moved this from Done to Highlights in Core development May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants