perf: hoist loop-invariant division out of reduce's cell loop - #439
Open
henryiii wants to merge 4 commits into
Open
perf: hoist loop-invariant division out of reduce's cell loop#439henryiii wants to merge 4 commits into
henryiii wants to merge 4 commits into
Conversation
Precompute the reduced axis extent once per axis instead of dividing per cell per axis, and skip the merge division when merge == 1. About 5-10% faster on large histograms (Apple clang -O3, M-series). Assisted-by: ClaudeCode:claude-fable-5
Collaborator
|
How was the benchmark done? Would be good to have the code. Is this incurring penalties for small histograms or is it a consistent win whatever the size / dimension? |
Benchmarks show the branch costs more than the division it skips: with the default unlimited_storage, shrink-only reduce was 5-9% slower than develop. Always dividing keeps the win from hoisting reduced_end and is never slower than develop in any measured case. Assisted-by: ClaudeCode:claude-fable-5
Contributor
Author
|
There was a small regression for Unlimited histograms of small size, fixed by avoiding branch (most recent commit). Code below (let me know if you want it somewhere in the library). 🤖 AI text below 🤖 Good questions - re-benchmarking to answer them found a real problem, now fixed in the latest push. Details: How the benchmark was done. Standalone translation units (code below), Apple clang 21 Small histograms. No penalty. The only per-call overhead is one loop over the axes to precompute What re-benchmarking found. The speedup as originally pushed reproduced with Results. develop vs. this PR (after the fix), range over 3 interleaved runs:
default
Small histograms ( bench_reduce.cpp (dense_storage, ns/cell)// Benchmark: algorithm::reduce per-cell loop cost.
#include <boost/histogram.hpp>
#include <boost/histogram/algorithm/reduce.hpp>
#include <chrono>
#include <cstdio>
#include <random>
#include <vector>
namespace bh = boost::histogram;
using namespace bh::algorithm;
using reg = bh::axis::regular<>;
int main() {
auto h3 = bh::make_histogram_with(bh::dense_storage<double>(), reg(100, 0, 1),
reg(100, 0, 1), reg(100, 0, 1));
auto h2 = bh::make_histogram_with(bh::dense_storage<double>(), reg(1000, 0, 1),
reg(1000, 0, 1));
std::default_random_engine gen(1);
std::uniform_real_distribution<double> dis(0, 1);
for (int i = 0; i < 1000000; ++i) h3(dis(gen), dis(gen), dis(gen));
for (int i = 0; i < 1000000; ++i) h2(dis(gen), dis(gen));
auto bench = [](const char* name, std::size_t ncells, auto&& run) {
double best = 1e30, total = 0;
for (int rep = 0; rep < 8; ++rep) {
auto t0 = std::chrono::high_resolution_clock::now();
total = run();
auto t1 = std::chrono::high_resolution_clock::now();
double dt = std::chrono::duration<double>(t1 - t0).count();
if (rep > 0 && dt < best) best = dt;
}
std::printf("%-30s %8.2f ns/cell (sum=%g)\n", name, best / ncells * 1e9, total);
};
const std::size_t n3 = 102ul * 102 * 102;
const std::size_t n2 = 1002ul * 1002;
bench("3d rebin(2,2,2)", n3, [&] {
auto r = reduce(h3, rebin(2), rebin(2), rebin(2));
return sum(r);
});
bench("3d shrink only (merge=1)", n3, [&] {
auto r = reduce(h3, shrink(0.1, 0.9), shrink(0.1, 0.9), shrink(0.1, 0.9));
return sum(r);
});
bench("2d rebin(2,2)", n2, [&] {
auto r = reduce(h2, rebin(2), rebin(2));
return sum(r);
});
bench("2d shrink only (merge=1)", n2, [&] {
auto r = reduce(h2, shrink(0.1, 0.9), shrink(0.1, 0.9));
return sum(r);
});
return 0;
}bench_reduce_sizes.cpp (default storage, size sweep incl. small histograms)// Benchmark for boost::histogram::algorithm::reduce (PR #439).
// Build: c++ -std=c++17 -O3 -DNDEBUG -I<histogram>/include -I<boost> bench_reduce.cpp
#include <boost/histogram.hpp>
#include <boost/histogram/algorithm/reduce.hpp>
#include <chrono>
#include <cstdio>
#include <random>
#include <vector>
using namespace boost::histogram;
using algorithm::rebin;
using algorithm::shrink;
using clk = std::chrono::steady_clock;
template <class H>
void fill_random(H& h) {
std::mt19937 gen(1);
std::uniform_real_distribution<> d(0, 10);
for (auto&& v : unsafe_access::storage(h)) v = d(gen);
}
// Run op() repeatedly for at least min_time; report best ns/call over reps.
template <class Op>
double bench(Op&& op) {
// warm-up and calibration
op();
auto t0 = clk::now();
op();
auto once = std::chrono::duration<double>(clk::now() - t0).count();
unsigned n = once > 0 ? static_cast<unsigned>(0.02 / once) + 1 : 1000;
double best = 1e300;
for (int rep = 0; rep < 7; ++rep) {
t0 = clk::now();
for (unsigned i = 0; i < n; ++i) op();
auto dt = std::chrono::duration<double>(clk::now() - t0).count() / n;
if (dt < best) best = dt;
}
return best * 1e9;
}
volatile unsigned sink;
template <class H, class Opts>
void run_case(const char* name, H& h, const Opts& opts) {
auto ns = bench([&] {
auto r = algorithm::reduce(h, opts);
sink += static_cast<unsigned>(r.size());
});
std::printf("%-28s %12.0f ns/call\n", name, ns);
}
int main() {
auto ax10 = axis::regular<>(10, 0, 1);
auto ax100 = axis::regular<>(100, 0, 1);
auto ax1000 = axis::regular<>(1000, 0, 1);
{ // 1d, 10 bins
auto h = make_histogram(ax10);
fill_random(h);
run_case("1d 10 rebin(2)", h, std::vector<algorithm::reduce_command>{rebin(0, 2)});
run_case("1d 10 shrink", h, std::vector<algorithm::reduce_command>{shrink(0, 0.2, 0.8)});
}
{ // 2d, 10x10
auto h = make_histogram(ax10, ax10);
fill_random(h);
run_case("2d 10x10 rebin(2)", h,
std::vector<algorithm::reduce_command>{rebin(0, 2), rebin(1, 2)});
run_case("2d 10x10 shrink", h,
std::vector<algorithm::reduce_command>{shrink(0, 0.2, 0.8), shrink(1, 0.2, 0.8)});
}
{ // 2d, 100x100
auto h = make_histogram(ax100, ax100);
fill_random(h);
run_case("2d 100x100 rebin(2)", h,
std::vector<algorithm::reduce_command>{rebin(0, 2), rebin(1, 2)});
run_case("2d 100x100 shrink", h,
std::vector<algorithm::reduce_command>{shrink(0, 0.2, 0.8), shrink(1, 0.2, 0.8)});
}
{ // 2d, 1000x1000 (1M cells)
auto h = make_histogram(ax1000, ax1000);
fill_random(h);
run_case("2d 1kx1k rebin(2)", h,
std::vector<algorithm::reduce_command>{rebin(0, 2), rebin(1, 2)});
run_case("2d 1kx1k shrink", h,
std::vector<algorithm::reduce_command>{shrink(0, 0.2, 0.8), shrink(1, 0.2, 0.8)});
}
{ // 3d, 10x10x10
auto h = make_histogram(ax10, ax10, ax10);
fill_random(h);
run_case("3d 10^3 rebin(2)", h,
std::vector<algorithm::reduce_command>{rebin(0, 2), rebin(1, 2), rebin(2, 2)});
run_case("3d 10^3 shrink", h,
std::vector<algorithm::reduce_command>{shrink(0, 0.2, 0.8), shrink(1, 0.2, 0.8),
shrink(2, 0.2, 0.8)});
}
{ // 3d, 100x100x100 (1M cells)
auto h = make_histogram(ax100, ax100, ax100);
fill_random(h);
run_case("3d 100^3 rebin(2)", h,
std::vector<algorithm::reduce_command>{rebin(0, 2), rebin(1, 2), rebin(2, 2)});
run_case("3d 100^3 shrink", h,
std::vector<algorithm::reduce_command>{shrink(0, 0.2, 0.8), shrink(1, 0.2, 0.8),
shrink(2, 0.2, 0.8)});
run_case("3d 100^3 mixed", h,
std::vector<algorithm::reduce_command>{rebin(0, 2), shrink(1, 0.2, 0.8),
rebin(2, 4)});
}
return 0;
} |
Collaborator
|
We have a benchmarking system in the repo, integrate those benchmarks in there and commit them along with the patch. |
Covers rebin, shrink, and shrink_and_rebin for 1d-3d histograms with dense_storage and unlimited_storage, 4-256 bins per axis. Assisted-by: ClaudeCode:claude-fable-5
Contributor
Author
|
🤖 AI text below 🤖 The benchmark is now integrated into the repo as |
Contributor
Author
|
Ahh, I did not ask for it to comment (or even push yet). Did you want cells per second as the report? Also there a a lot of parameterizations, I can cut them down a bit if you'd prefer. Here's what it looks like when I run it manually on my M5Pro: |
Contributor
Author
|
Looking at the benchmarks, I think I can improve on this a little for very small histograms, will push a slightly better version if it does benchmark better in a few minutes. |
Removes the extra per-axis pass before the cell loop; the values are now set in the axes_transform lambda, which already visits every axis. Assisted-by: ClaudeCode:claude-fable-5
Contributor
Author
|
Slightly improved version committed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is fairly small change for a nice small speedup in reduction. From #437.
🤖 AI text below 🤖
reducerecomputed(end - begin) / mergefor every axis of every cell. This precomputes the reduced extent once per axis in the command buffer.Benchmarked on 1M-cell histograms (Apple clang -O3, M-series; both branches compiled separately, binaries run interleaved): with
dense_storage<double>, 3d rebin/shrink are ~6-11% faster and 2d rebin ~4% faster; with the defaultunlimited_storageand for small histograms, no change (never slower than develop in any measured case). Benchmark code and full numbers are in the comments below.An earlier version of this PR also skipped the division when
merge == 1; that branch turned out to cost more than the division it avoids and was removed.