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.

Sign up for GitHub

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

Jump to bottom

Do not waste time constructing error string if nobody is interested in it in canTransform(). #469

Merged
merged 1 commit into from
Aug 24, 2020
Merged

Do not waste time constructing error string if nobody is interested in it in canTransform(). #469

merged 1 commit into from
Aug 24, 2020

Conversation

peci1
Copy link
Contributor

@peci1 peci1 commented Jun 18, 2020

Solves #468.

Also added two more tests to speed_test to verify that this patch helps.

@peci1 peci1 requested a review from tfoote as a code owner June 18, 2020 15:03
@peci1
Copy link
Contributor Author

peci1 commented Jun 18, 2020

Speed test before patch:

Info:    9 to 4
Info:    Doing 1000000 10-level 1.000000-interval tests
Info:    canTransform at Time(3) without error string took 2.520531 for an average of 0.000002521
Info:    canTransform at Time(3) with error string took 4.727922 for an average of 0.000004728

Speed test after patch:

Info:    9 to 4
Info:    Doing 1000000 10-level 1.000000-interval tests
Info:    canTransform at Time(3) without error string took 0.221355 for an average of 0.000000221
Info:    canTransform at Time(3) with error string took 4.029124 for an average of 0.000004029

That's tenfold speedup!

Other cases (when the transform exists) are without change.

This was referenced Jun 18, 2020
Open
Closed
tfoote
tfoote approved these changes Jun 19, 2020
View reviewed changes
Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

Thanks this is a great improvement.

@tfoote tfoote merged commit 34a4154 into ros:noetic-devel Aug 24, 2020
@peci1
Copy link
Contributor Author

peci1 commented Aug 24, 2020

Thanks for merging. Will you pick this up to backport into Melodic, please?

@jlack1987 jlack1987 mentioned this pull request Jul 21, 2022
Open
ooeygui pushed a commit to ms-iot/geometry2 that referenced this pull request Oct 12, 2022
@aprotyas
tf2: Enable common linter tests (ros#469)
9ff7dad 
As discussed in ros2/geometry2#467:

- Enables a subset of relevant linter tests from `ament_lint_common` in `tf2`.
- Excludes LinearMath headers (include/tf2/LinearMath) from being linted.
- Fixes line-length formatting issues in a few source files, so that there are no lint failures in the current state.

Note that individual linters had to be invoked rather than use `ament_lint_auto` because the LinearMath headers should not have linters run on them - see ros2/geometry2#258 (comment) for more information - and because `ament_lint_auto` does not provide a general way of providing file exclusion lists.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tfoote tfoote tfoote approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@peci1 @tfoote