cassie_bench: Improve allocation tracking #13930
Conversation
There was a problem hiding this comment.
+@sherm1 for feature review.
FYI @jwnimmer-tri hopefully friendly amendment to LimitMalloc
FYI @joemasterjohn general benchmarking interest
Reviewable status: LGTM missing from assignee sherm1(platform), needs at least two assigned reviewers (waiting on @sherm1)
There was a problem hiding this comment.
Reviewable status: LGTM missing from assignee sherm1(platform), needs at least two assigned reviewers (waiting on @sherm1)
a discussion (no related file):
Example output. Not a fully controlled experiment, but shouidn't matter for malloc counting:
cd ~/checkout/drake && bazel run //examples/multibody/cassie_benchmark:cassie_bench -- --benchmark_counters_tabular=true
Loading:
Loading: 0 packages loaded
Analyzing: target //examples/multibody/cassie_benchmark:cassie_bench (0 packages loaded, 0 targets configured)
INFO: Analyzed target //examples/multibody/cassie_benchmark:cassie_bench (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
bazel: Entering directory `/home/rico/.cache/bazel/_bazel_rico/7f8997f28c9253517a55d673c67a6c74/execroot/drake/'
[0 / 1] [Prepa] BazelWorkspaceStatusAction stable-status.txt
[1 / 4] Compiling examples/multibody/cassie_benchmark/test/cassie_bench.cc; 1s linux-sandbox
[2 / 4] [Prepa] Linking examples/multibody/cassie_benchmark/cassie_bench
bazel: Leaving directory `/home/rico/.cache/bazel/_bazel_rico/7f8997f28c9253517a55d673c67a6c74/execroot/drake/'
Target //examples/multibody/cassie_benchmark:cassie_bench up-to-date:
bazel-bin/examples/multibody/cassie_benchmark/cassie_bench
INFO: Elapsed time: 11.030s, Critical Path: 10.73s
INFO: 2 processes: 2 linux-sandbox.
INFO: Build completed successfully, 3 total actions
INFO: Running command line: external/bazel_tools/tools/test/test-setup.sh examples/multibody/cassie_benchmark/cassie_bench '--benchmark_counters_tabular=true'
INFO: Build completed successfully, 3 total actions
exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //examples/multibody/cassie_benchmark:cassie_bench
-----------------------------------------------------------------------------
2020-08-25 17:43:34
Running /home/rico/.cache/bazel/_bazel_rico/7f8997f28c9253517a55d673c67a6c74/execroot/drake/bazel-out/k8-opt/bin/examples/multibody/cassie_benchmark/cassie_bench.runfiles/drake/examples/multibody/cassie_benchmark/cassie_bench
Run on (48 X 3500 MHz CPU s)
CPU Caches:
L1 Data 32 KiB (x24)
L1 Instruction 32 KiB (x24)
L2 Unified 256 KiB (x24)
L3 Unified 30720 KiB (x2)
Load Average: 0.45, 0.79, 0.62
***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead.
----------------------------------------------------------------------------------------------------------------------------------------
Benchmark Time CPU Iterations Allocs.max Allocs.mean Allocs.min Allocs.stddev
----------------------------------------------------------------------------------------------------------------------------------------
CassieDoubleFixture/DoubleMassMatrix 26524 ns 26523 ns 24196 174 174 174 0
CassieDoubleFixture/DoubleInverseDynamics 19784 ns 19783 ns 35404 3 3 3 0
CassieDoubleFixture/DoubleForwardDynamics 40148 ns 40147 ns 17287 22 22 22 0
CassieAutodiffFixture/AutodiffMassMatrix 4171999 ns 4171941 ns 166 61.197k 61.197k 61.197k 0
CassieAutodiffFixture/AutodiffInverseDynamics 5097747 ns 5097673 ns 100 68.798k 68.798k 68.798k 0
CassieAutodiffFixture/AutodiffForwardDynamics 8028023 ns 8027747 ns 93 102.904k 102.904k 102.904k 0 ```
There was a problem hiding this comment.
Reviewable status: LGTM missing from assignee sherm1(platform), needs at least two assigned reviewers (waiting on @sherm1)
a discussion (no related file):
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
Example output. Not a fully controlled experiment, but shouidn't matter for malloc counting:
cd ~/checkout/drake && bazel run //examples/multibody/cassie_benchmark:cassie_bench -- --benchmark_counters_tabular=true Loading: Loading: 0 packages loaded Analyzing: target //examples/multibody/cassie_benchmark:cassie_bench (0 packages loaded, 0 targets configured) INFO: Analyzed target //examples/multibody/cassie_benchmark:cassie_bench (0 packages loaded, 0 targets configured). INFO: Found 1 target... bazel: Entering directory `/home/rico/.cache/bazel/_bazel_rico/7f8997f28c9253517a55d673c67a6c74/execroot/drake/' [0 / 1] [Prepa] BazelWorkspaceStatusAction stable-status.txt [1 / 4] Compiling examples/multibody/cassie_benchmark/test/cassie_bench.cc; 1s linux-sandbox [2 / 4] [Prepa] Linking examples/multibody/cassie_benchmark/cassie_bench bazel: Leaving directory `/home/rico/.cache/bazel/_bazel_rico/7f8997f28c9253517a55d673c67a6c74/execroot/drake/' Target //examples/multibody/cassie_benchmark:cassie_bench up-to-date: bazel-bin/examples/multibody/cassie_benchmark/cassie_bench INFO: Elapsed time: 11.030s, Critical Path: 10.73s INFO: 2 processes: 2 linux-sandbox. INFO: Build completed successfully, 3 total actions INFO: Running command line: external/bazel_tools/tools/test/test-setup.sh examples/multibody/cassie_benchmark/cassie_bench '--benchmark_counters_tabular=true' INFO: Build completed successfully, 3 total actions exec ${PAGER:-/usr/bin/less} "$0" || exit 1 Executing tests from //examples/multibody/cassie_benchmark:cassie_bench ----------------------------------------------------------------------------- 2020-08-25 17:43:34 Running /home/rico/.cache/bazel/_bazel_rico/7f8997f28c9253517a55d673c67a6c74/execroot/drake/bazel-out/k8-opt/bin/examples/multibody/cassie_benchmark/cassie_bench.runfiles/drake/examples/multibody/cassie_benchmark/cassie_bench Run on (48 X 3500 MHz CPU s) CPU Caches: L1 Data 32 KiB (x24) L1 Instruction 32 KiB (x24) L2 Unified 256 KiB (x24) L3 Unified 30720 KiB (x2) Load Average: 0.45, 0.79, 0.62 ***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead. ---------------------------------------------------------------------------------------------------------------------------------------- Benchmark Time CPU Iterations Allocs.max Allocs.mean Allocs.min Allocs.stddev ---------------------------------------------------------------------------------------------------------------------------------------- CassieDoubleFixture/DoubleMassMatrix 26524 ns 26523 ns 24196 174 174 174 0 CassieDoubleFixture/DoubleInverseDynamics 19784 ns 19783 ns 35404 3 3 3 0 CassieDoubleFixture/DoubleForwardDynamics 40148 ns 40147 ns 17287 22 22 22 0 CassieAutodiffFixture/AutodiffMassMatrix 4171999 ns 4171941 ns 166 61.197k 61.197k 61.197k 0 CassieAutodiffFixture/AutodiffInverseDynamics 5097747 ns 5097673 ns 100 68.798k 68.798k 68.798k 0 CassieAutodiffFixture/AutodiffForwardDynamics 8028023 ns 8027747 ns 93 102.904k 102.904k 102.904k 0 ``` </blockquote></details> FTR, the allocation numbers are from a dev branch with 13928 merged in, so won't be reproducible for a while yet. --- <!-- Sent from Reviewable.io -->
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee sherm1(platform), needs at least two assigned reviewers (waiting on @rpoyner-tri and @sherm1)
a discussion (no related file):
TODO: re-check the allocation limits if this gets rebased on improvements to multibody code.
There was a problem hiding this comment.
LGTM on LimitMalloc patch so far. Please do add a one-liner unit test call though, to make sure it keeps working (in https://github.com/RobotLocomotion/drake/blob/master/common/test_utilities/test/limit_malloc_test.cc).
Reviewed 2 of 3 files at r1.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee sherm1(platform), needs at least two assigned reviewers (waiting on @rpoyner-tri and @sherm1)
rpoyner-tri
force-pushed
the
malloc-dev
branch
from
e6cd546
to
5c78ab4
Compare
There was a problem hiding this comment.
Feature 
Reviewed 3 of 3 files at r1, 1 of 1 files at r2.
Reviewable status: 2 unresolved discussions, needs at least two assigned reviewers (waiting on @rpoyner-tri)
a discussion (no related file):
Previously, rpoyner-tri (Rick Poyner (rico)) wrote…
FTR, the allocation numbers are from a dev branch with 13928 merged in, so won't be reproducible for a while yet.
minor: the allocations should be reported as integers rather than floating point numbers since we need to know them exactly to set the limits.
examples/multibody/cassie_benchmark/test/cassie_bench.cc, line 116 at r2 (raw file):
return std::sqrt(m2_ / (updates_ - 1)); } }();
btw this would be shorter & clearer as a "?:" expression:
= updates_ < 2 ? std::numeric_limits<double>::quiet_NaN()
: std::sqrt(m2_ / (updates_ - 1));examples/multibody/cassie_benchmark/test/cassie_bench.cc, line 122 at r2 (raw file):
min_ = std::min(min_, allocs); max_ = std::max(max_, allocs); updates_++;
btw consider preferring preincrement when post not required for its semantics. It doesn't matter for integer types but does for iterators (see style guide). As written reviewer has to go look for the type to see whether this is allowed. (Not a defect here) Always using preincrement avoids trouble.
There was a problem hiding this comment.
Reviewable status: 3 unresolved discussions, needs at least two assigned reviewers (waiting on @rpoyner-tri)
a discussion (no related file):
Previously, sherm1 (Michael Sherman) wrote…
minor: the allocations should be reported as integers rather than floating point numbers since we need to know them exactly to set the limits.
That's not something that's easy to get from the framework. I can look into it a bit more.
rpoyner-tri
force-pushed
the
malloc-dev
branch
from
5c78ab4
to
2e0e3bc
Compare
There was a problem hiding this comment.
+@jwnimmer-tri for platform review on Wednesday schedule.
Reviewable status: LGTM missing from assignee jwnimmer-tri(platform) (waiting on @sherm1)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, 1 of 1 files at r3.
Reviewable status: 1 unresolved discussion (waiting on @rpoyner-tri)
examples/multibody/cassie_benchmark/test/cassie_bench.cc, line 125 at r3 (raw file):
private: benchmark::State* state_;
nit Per Drake GSG needs either a {} or the unreassignable flavor of const-ness.
* expose allocation count from LimitMalloc * track allocations with some simple stats * separate first run to expose steady state allocation * lower some Limitmalloc ceilings from observations
rpoyner-tri
force-pushed
the
malloc-dev
branch
from
2e0e3bc
to
f3ec2f3
Compare
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4.
Reviewable status:complete! all discussions resolved, LGTM from assignees jwnimmer-tri(platform),sherm1(platform)
This change is