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

Benchmark addon allows printing results as json file. #7

Merged
merged 2 commits into from
Apr 14, 2020

Conversation

martinfelis
Copy link
Collaborator

  • Should enable tracking of performance regressions.
  • Existing dynamics and contacts benchmarks supported (not yet: kinematics benchmarks)

- Should enable tracking of performance regressions.
- Existing dynamics and contacts benchmarks supported (not yet: kinematics benchmarks)
@martinfelis martinfelis linked an issue Mar 29, 2020 that may be closed by this pull request
Copy link
Member

@mkudruss mkudruss left a comment

Choose a reason for hiding this comment

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

Hi Martin,
thanks for the contributions. Json output will be very helpful in the future for performance regression analysis. Here are some general comments on your changes:

  • CMakeLists.txt <- No comments.
  • SampleData.h <- No problem with the additions but the file needs some reformatting to pass RBDL style guide. Spacing around ctor, dtor, etc. would make it better readable. Consider doing the changes for this file.
  • benchmarks.cc <- Code is slightly refactored. The execution of benchmarks should be more generalized in the future for better maintainability. Same holds true for the manual composition of the json output.
  • SimpleMath.h <- Nice that you added the eigen3 operators for simple math. Indeed helpful. Consider adding some "consts" when for read only variables.
  • rbdl_version.cc <- No problem with the code but needs some reformatting to pass RBDL style guide. This is also true for your changes. Consider making the changes for the whole file.

src/rbdl_version.cc Outdated Show resolved Hide resolved
include/rbdl/SimpleMath/SimpleMath.h Outdated Show resolved Hide resolved
include/rbdl/SimpleMath/SimpleMath.h Outdated Show resolved Hide resolved
include/rbdl/SimpleMath/SimpleMath.h Outdated Show resolved Hide resolved
include/rbdl/SimpleMath/SimpleMath.h Outdated Show resolved Hide resolved
addons/benchmark/SampleData.h Outdated Show resolved Hide resolved
addons/benchmark/benchmark.cc Show resolved Hide resolved
addons/benchmark/benchmark.cc Outdated Show resolved Hide resolved
addons/benchmark/benchmark.cc Show resolved Hide resolved
@mkudruss
Copy link
Member

@paulmanns will you perform the review or can we close the PR?

Copy link

@paulmanns paulmanns left a comment

Choose a reason for hiding this comment

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

Approved.

@martinfelis martinfelis merged commit d7846f7 into rbdl:dev Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add json output to the benchmark utility
3 participants