Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Roslaunch performance improvement#682

Merged
dirk-thomas merged 2 commits intoindigo-develfrom
roslaunch_performance_improvement
Oct 13, 2015
Merged

Roslaunch performance improvement#682
dirk-thomas merged 2 commits intoindigo-develfrom
roslaunch_performance_improvement

Conversation

@dirk-thomas
Copy link
Copy Markdown
Member

@dirk-thomas dirk-thomas commented Oct 12, 2015

This reverts #676 and implements a different approach.

@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Oct 12, 2015

+1

@dirk-thomas dirk-thomas mentioned this pull request Oct 12, 2015
Merged
@ros-pull-request-builder
Copy link
Copy Markdown
Member

ros-pull-request-builder commented Oct 12, 2015

Test passed.
Refer to this link for build results: http://jenkins.ros.org/job/_pull_request-indigo-ros_comm/406/

wkentaro
wkentaro reviewed Oct 12, 2015
View reviewed changes
tools/roslaunch/src/roslaunch/rlutil.py
Copy link
Copy Markdown
Contributor

@wkentaro wkentaro Oct 12, 2015

Choose a reason for hiding this comment

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

Can we use cached resource for find_resource? (not only find_node at https://github.com/ros/ros_comm/pull/682/files#diff-67998749e41c63572aa0901b6715e9a0R251)

Copy link
Copy Markdown
Member Author

@dirk-thomas dirk-thomas Oct 12, 2015

Choose a reason for hiding this comment

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

This function seems to be only called once. What is your use case where caching data here would improve anything?

Copy link
Copy Markdown
Contributor

@wkentaro wkentaro Oct 12, 2015

Choose a reason for hiding this comment

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

Actually I don't fully understand about rospack instance. I just wonder we can pass rospack instance to find_resource function which has cached source _path_to_package as previous implementation.

@wkentaro
Copy link
Copy Markdown
Contributor

wkentaro commented Oct 12, 2015

The finding paths is really fast. It's even faster than the implementation before in #676.

@ros-pull-request-builder
Copy link
Copy Markdown
Member

ros-pull-request-builder commented Oct 12, 2015

Test passed.
Refer to this link for build results: http://jenkins.ros.org/job/_pull_request-indigo-ros_comm/409/

@dirk-thomas dirk-thomas force-pushed the roslaunch_performance_improvement branch from fa79ca8 to f2a5838 Compare October 13, 2015 00:48
dirk-thomas added a commit that referenced this pull request Oct 13, 2015
@dirk-thomas
Merge pull request #682 from ros/roslaunch_performance_improvement
ec86fc0 
Roslaunch performance improvement
@dirk-thomas dirk-thomas merged commit ec86fc0 into indigo-devel Oct 13, 2015
@dirk-thomas dirk-thomas deleted the roslaunch_performance_improvement branch October 13, 2015 00:49
@ros-pull-request-builder
Copy link
Copy Markdown
Member

ros-pull-request-builder commented Oct 13, 2015

Test passed.
Refer to this link for build results: http://jenkins.ros.org/job/_pull_request-indigo-ros_comm/410/

@ros-pull-request-builder
Copy link
Copy Markdown
Member

ros-pull-request-builder commented Oct 13, 2015

Test passed.
Refer to this link for build results: http://jenkins.ros.org/job/_pull_request-indigo-ros_comm/413/

@guihomework
Copy link
Copy Markdown

guihomework commented Oct 14, 2015

@dirk-thomas , thanks a lot on solving this caching problem.

However, we encountered a similar problem also in gazebo_ros_pkgs (ros-simulation/gazebo_ros_pkgs#362) when resolving URDF resources (package://)
Indeed, the ros::package::getPath() is used there and is not caching anything either. We cached locally now which makes the resource resolution 6 times faster, but it might make sense to cache in the ros::package implementation. That caching at low level would probably also accelerate rviz loading models, and probably resolving URDF resources the same way.

@dirk-thomas dirk-thomas mentioned this pull request Nov 25, 2015
Closed
@k-okada k-okada mentioned this pull request Nov 30, 2015
Closed
@dirk-thomas dirk-thomas mentioned this pull request Jan 26, 2017
Merged
rsinnet pushed a commit to MisoRobotics/ros_comm that referenced this pull request Jun 19, 2017
@dirk-thomas
Merge pull request ros#682 from ros/roslaunch_performance_improvement
9ca1b8e 
Roslaunch performance improvement
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dirk-thomas @wjwwood @ros-pull-request-builder @wkentaro @guihomework