-
Notifications
You must be signed in to change notification settings - Fork 27
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
Adds initial benchmarking for axom::Array vs. std::vector #1469
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, looks good. I commented with a couple of questions.
bda418b
to
9bf3442
Compare
Oof, those numbers for primitive types are not pretty. Did you see anything obvious popping out that might’ve been the cause @kennyweiss? |
I haven't had a chance to dig too deeply -- this PR is mostly shining a light on the problem. |
|
||
// Custom fmt formatter for ArrayFeatureBenchmarks | ||
template <> | ||
struct axom::fmt::formatter<ArrayFeatureBenchmarks> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised fmt requires this much code to print an enum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed -- it's because I'm using the enum for flags and wanted to print all enabled features, e.g.
>./tests/core_benchmark_array -f constructors insertion
[INFO] Parsed and processed command line arguments:
[INFO] - Array sizes: 65536
[INFO] - Array features to test: Constructors|Insertion # <-- Output for two flags
...
>./tests/core_benchmark_array -f constructors insertion -f all
[INFO] Parsed and processed command line arguments:
[INFO] - Array sizes: 65536
[INFO] - Array features to test: All # <-- Output for "all" flags
Happy to improve it if you know of a better way.
// clang-format off | ||
if((args_benchmark_features & ArrayFeatureBenchmarks::Constructors) != ArrayFeatureBenchmarks::None) | ||
{ | ||
benchmark::RegisterBenchmark(tname("Array::ctor"), &ctor<axom::Array<T>>)->Apply(CustomArgs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking the address of the functions surprised me here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's because it's registering it at runtime, so needs a function pointer rather than using a macro (?)
@@ -161,6 +161,7 @@ void Shaper::loadShapeInternal(const klee::Shape& shape, | |||
double& revolvedVolume) | |||
{ | |||
using axom::utilities::string::endsWith; | |||
AXOM_UNUSED_VAR(percentError); // only currently used with C2C |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing - I think I caused that warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Allows users to control which array features to test as well as the sizes of the arrays.
It was taking about 2 minutes before and is now taking less than a second
Use Array::reserve rather than Array::resize for initial allocation
4b89107
to
f7f6a25
Compare
Avoided calling `typeid` on a pointer type to avoid `-Wpotentially-evaluated-expression`.
f7f6a25
to
3d0fbf2
Compare
Summary
(see also Benchmark
axom::Array
performance #922)axom::Array
vs.std::vector
push_back
andemplace_back
and some iterator operations.slic
andslam
ENABLE_BENCHMARKS
to a debug and a release config CI job and runs the benchmarks as part of the testingDetails
Here are the initial results for a Release Clang config on an LC CTS-2 cluster comparing
axom::Array
vs.std::vector
templates on several types:int
std::pair<int,int>
Wrapper<int>
-- a struct that wraps anint
std::string
For simplicity, these results only use a single array size --$2^{16} == 65,536$ elements, although it's easy to test other sizes and/or types.
My quick read is that
std::vector
can be several times faster thanaxom::Array
forpush_back
andemplace_back
on simple types, even when we reserve storage ahead of time (compare e.g. the lines withpush_back_initialReserve
)