Skip to content

Commit cba92f7

Browse files
Optimize fusion moves
1 parent c96541e commit cba92f7

12 files changed

Lines changed: 538 additions & 105 deletions

File tree

AGENTS.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,25 @@ Tests run against the installed Python package. For each public function, cover:
120120
121121
Correct first, fast second. Benchmark before adding complexity. Prefer algorithmic improvements over build-level tweaks. Release the GIL for expensive kernels. Add threading only after the single-threaded implementation is stable; it must be portable and user-controllable.
122122
123+
### Profiling
124+
125+
Measure before optimizing. The codebase carries a lightweight per-phase profiling utility for exactly this:
126+
127+
- Header: `include/bioimage_cpp/detail/profile.hxx`. Macros: `BIOIMAGE_PROFILE_INIT(name)`, `BIOIMAGE_PROFILE_SCOPE(name, "label")`, `BIOIMAGE_PROFILE_REPORT(name)`.
128+
- Gated behind the `BIOIMAGE_PROFILE` compile-time flag. Outside of profile builds the macros expand to no-ops and a `NullProfiler` stub so the same code compiles unchanged.
129+
- Enable via CMake option: `pip install -e . --no-build-isolation -C cmake.define.BIOIMAGE_PROFILE=ON`. Rebuild without the flag for production work.
130+
- Reports per-phase wall-clock totals to stderr at the end of the instrumented scope (e.g., at the end of `optimize`). Same labels accumulate across multiple invocations of the scope (e.g., per-iteration phases).
131+
132+
Workflow when adding or chasing a performance issue:
133+
134+
1. **Compare standalone primitives first** (`development/.../check_*.py` scripts vs. nifty). If a primitive is already fast, the gap is elsewhere — don't optimize it speculatively.
135+
2. **Instrument the suspect function** by wrapping each logical phase in a `BIOIMAGE_PROFILE_SCOPE`. Pick labels that map to one operation each ("agreement_contract", "sub_solve", "energy_eval", ...), not full call paths.
136+
3. **Build with `BIOIMAGE_PROFILE=ON` and run a realistic problem** — typically the external multicut instance loaded by the comparison scripts. Run with `--repeats 1` so the report isn't drowned out.
137+
4. **Optimize the largest phase** (50% phase beats two 10% phases combined). Re-measure after each change; verify no other phase regressed.
138+
5. **Strip the instrumentation when done** only if it adds clutter; otherwise leave it in place — it's free when the flag is off.
139+
140+
Don't add `std::chrono` snippets ad hoc; use the existing macros so future profiling sessions land in a consistent format.
141+
123142
## Documentation
124143
125144
`README.md` covers: what `bioimage-cpp` is and isn't, install/build, minimal examples, design philosophy. Public functions need concise docstrings documenting input shapes, supported dtypes, output shapes/dtypes, copy behavior, background-label behavior (if relevant), and axis/coordinate conventions.

CMakeLists.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,9 @@ target_compile_options(_core PRIVATE
2727
$<$<NOT:$<CONFIG:Debug>>:-O3>
2828
)
2929

30+
option(BIOIMAGE_PROFILE "Enable per-phase profiling instrumentation (development only)" OFF)
31+
if(BIOIMAGE_PROFILE)
32+
target_compile_definitions(_core PRIVATE BIOIMAGE_PROFILE)
33+
endif()
34+
3035
install(TARGETS _core LIBRARY DESTINATION bioimage_cpp)

include/bioimage_cpp/detail/indexed_heap.hxx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,28 @@ public:
133133
sift_up(pos);
134134
}
135135

136+
// Bulk-load the heap from a list of entries in O(n) via Floyd's heapify.
137+
// Replaces any current heap contents. Preconditions:
138+
// - the heap is empty before this call (call `clear()`/`reset_capacity`
139+
// first if needed);
140+
// - every key in `entries` is unique.
141+
// Compared to N successive `push` calls (O(n log n)) this matters when
142+
// initializing a heap from a large edge set in one shot.
143+
void build_heap(std::vector<Entry> entries) {
144+
heap_ = std::move(entries);
145+
for (std::size_t pos = 0; pos < heap_.size(); ++pos) {
146+
locator_.set(heap_[pos].key, pos);
147+
}
148+
if (heap_.size() < 2) {
149+
return;
150+
}
151+
// Sift down from the last internal node back to the root. After each
152+
// sift_down, the subtree rooted at `pos` is heap-ordered.
153+
for (std::size_t pos = heap_.size() / 2; pos-- > 0;) {
154+
sift_down(pos);
155+
}
156+
}
157+
136158
// Precondition: `key` is currently in the heap.
137159
void change(const KeyT &key, PriorityT priority) {
138160
const auto pos = locator_.at(key);
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
#pragma once
2+
3+
// Lightweight per-phase profiling utility. Active only when the translation
4+
// unit is compiled with `-DBIOIMAGE_PROFILE`; otherwise every macro is a no-op
5+
// and adds no overhead. Use it in development/profile-mode builds to find
6+
// hotspots; do not enable it in production wheels.
7+
8+
#ifdef BIOIMAGE_PROFILE
9+
#include <chrono>
10+
#include <cstdio>
11+
#include <string>
12+
#include <unordered_map>
13+
#include <vector>
14+
15+
namespace bioimage_cpp::detail {
16+
17+
class Profiler {
18+
public:
19+
using Clock = std::chrono::steady_clock;
20+
using Duration = std::chrono::duration<double>;
21+
22+
void accumulate(const char *name, const Duration delta) {
23+
auto it = totals_.find(name);
24+
if (it == totals_.end()) {
25+
order_.push_back(name);
26+
totals_.emplace(name, delta.count());
27+
} else {
28+
it->second += delta.count();
29+
}
30+
}
31+
32+
void report() const {
33+
std::fprintf(stderr, "[bioimage profile]\n");
34+
double total = 0.0;
35+
for (const auto *name : order_) {
36+
total += totals_.at(name);
37+
}
38+
for (const auto *name : order_) {
39+
const auto seconds = totals_.at(name);
40+
const auto fraction = total > 0.0 ? (100.0 * seconds / total) : 0.0;
41+
std::fprintf(stderr, " %-22s %8.4f s (%5.1f%%)\n", name, seconds, fraction);
42+
}
43+
std::fprintf(stderr, " %-22s %8.4f s\n", "total", total);
44+
}
45+
46+
private:
47+
std::vector<const char *> order_;
48+
std::unordered_map<const char *, double> totals_;
49+
};
50+
51+
class ProfileTimer {
52+
public:
53+
ProfileTimer(Profiler &profiler, const char *name)
54+
: profiler_(profiler), name_(name), start_(Profiler::Clock::now()) {}
55+
56+
~ProfileTimer() {
57+
profiler_.accumulate(name_, Profiler::Clock::now() - start_);
58+
}
59+
60+
private:
61+
Profiler &profiler_;
62+
const char *name_;
63+
Profiler::Clock::time_point start_;
64+
};
65+
66+
} // namespace bioimage_cpp::detail
67+
68+
#define BIOIMAGE_PROFILE_INIT(var) ::bioimage_cpp::detail::Profiler var;
69+
#define BIOIMAGE_PROFILE_SCOPE(var, name) ::bioimage_cpp::detail::ProfileTimer _bp_##__LINE__(var, name);
70+
#define BIOIMAGE_PROFILE_REPORT(var) (var).report();
71+
72+
#else
73+
74+
namespace bioimage_cpp::detail {
75+
struct NullProfiler {};
76+
} // namespace bioimage_cpp::detail
77+
78+
#define BIOIMAGE_PROFILE_INIT(var) ::bioimage_cpp::detail::NullProfiler var;
79+
#define BIOIMAGE_PROFILE_SCOPE(var, name) (void)var;
80+
#define BIOIMAGE_PROFILE_REPORT(var) (void)var;
81+
82+
#endif

include/bioimage_cpp/detail/union_find.hxx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,16 @@ public:
7070
return parents_.size();
7171
}
7272

73+
// Re-initialise the union-find to `n` singletons. Reuses the existing
74+
// vectors' capacity when possible, so the workspace pattern (reusing a
75+
// single `UnionFind` across many solver invocations on graphs of varying
76+
// size) avoids reallocations.
77+
void reset(const std::size_t n) {
78+
parents_.resize(n);
79+
ranks_.assign(n, 0);
80+
std::iota(parents_.begin(), parents_.end(), std::uint64_t{0});
81+
}
82+
7383
private:
7484
std::vector<std::uint64_t> parents_;
7585
std::vector<std::uint64_t> ranks_;

include/bioimage_cpp/graph/detail/fusion_contract.hxx

Lines changed: 79 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
#pragma once
22

3-
#include "bioimage_cpp/detail/relabel.hxx"
43
#include "bioimage_cpp/detail/union_find.hxx"
54
#include "bioimage_cpp/graph/undirected_graph.hxx"
65

6+
#include <algorithm>
77
#include <cstddef>
88
#include <cstdint>
9+
#include <limits>
910
#include <stdexcept>
1011
#include <string>
12+
#include <utility>
1113
#include <vector>
1214

1315
namespace bioimage_cpp::graph::detail {
@@ -30,6 +32,17 @@ struct AgreementContraction {
3032
// `proposals` is a row-major buffer of shape (n_proposals, number_of_nodes).
3133
// Passing 0 proposals collapses every edge (all proposals trivially agree)
3234
// and yields a single-node contracted graph; we reject that as a usage error.
35+
//
36+
// Implementation notes:
37+
// - Dense relabeling uses a flat sentinel array indexed by root id rather
38+
// than a hash map; roots live in [0, number_of_nodes) so this is O(N).
39+
// - Surviving (min_root, max_root) pairs are collected and sorted by a
40+
// packed 64-bit key, then deduped sequentially. The contracted graph is
41+
// built in one pass via `UndirectedGraph::from_sorted_unique_edges`,
42+
// bypassing per-edge hash insertion in `insert_edge`.
43+
// - The contracted graph's `edge_lookup_` is left empty because the
44+
// fusion-move sub-solver only iterates edges and adjacency, never calls
45+
// `find_edge`.
3346
inline AgreementContraction contract_by_agreement(
3447
const UndirectedGraph &graph,
3548
const std::uint64_t *proposals,
@@ -67,36 +80,87 @@ inline AgreementContraction contract_by_agreement(
6780
}
6881
}
6982

70-
std::vector<std::uint64_t> raw_root(static_cast<std::size_t>(number_of_nodes));
83+
// Dense-relabel UFD roots in one O(N) pass with a sentinel array.
84+
constexpr std::uint64_t unset = std::numeric_limits<std::uint64_t>::max();
85+
std::vector<std::uint64_t> dense_of_raw(
86+
static_cast<std::size_t>(number_of_nodes), unset
87+
);
88+
std::vector<std::uint64_t> root_of_node(static_cast<std::size_t>(number_of_nodes));
89+
std::uint64_t number_of_components = 0;
7190
for (std::uint64_t node = 0; node < number_of_nodes; ++node) {
72-
raw_root[static_cast<std::size_t>(node)] = sets.find(node);
91+
const auto raw = sets.find(node);
92+
auto dense = dense_of_raw[static_cast<std::size_t>(raw)];
93+
if (dense == unset) {
94+
dense = number_of_components++;
95+
dense_of_raw[static_cast<std::size_t>(raw)] = dense;
96+
}
97+
root_of_node[static_cast<std::size_t>(node)] = dense;
7398
}
74-
auto root_of_node = bioimage_cpp::detail::dense_relabel(raw_root);
7599

76-
std::uint64_t number_of_components = 0;
77-
for (const auto root : root_of_node) {
78-
if (root + 1 > number_of_components) {
79-
number_of_components = root + 1;
80-
}
100+
// Sort key for surviving edges. The lower 32 bits hold `max_root` and
101+
// the upper 32 bits hold `min_root` so a single uint64 comparison
102+
// suffices. This requires number_of_components to fit in 32 bits, which
103+
// is always true for graphs we can fit in memory.
104+
if (number_of_components > (std::uint64_t{1} << 32)) {
105+
throw std::runtime_error(
106+
"number_of_components exceeds 2^32 — contraction packing assumption violated"
107+
);
81108
}
82109

83-
UndirectedGraph contracted_graph(number_of_components);
110+
struct Survivor {
111+
std::uint64_t key; // (min_root << 32) | max_root
112+
std::uint64_t original_edge;
113+
};
114+
std::vector<Survivor> survivors;
115+
survivors.reserve(static_cast<std::size_t>(number_of_edges));
116+
84117
std::vector<std::int64_t> contracted_edge_of_original(
85118
static_cast<std::size_t>(number_of_edges), -1
86119
);
87120

88121
for (std::uint64_t edge = 0; edge < number_of_edges; ++edge) {
89122
const auto uv = graph.uv(edge);
90-
const auto ru = root_of_node[static_cast<std::size_t>(uv.first)];
91-
const auto rv = root_of_node[static_cast<std::size_t>(uv.second)];
123+
auto ru = root_of_node[static_cast<std::size_t>(uv.first)];
124+
auto rv = root_of_node[static_cast<std::size_t>(uv.second)];
92125
if (ru == rv) {
93126
continue;
94127
}
95-
const auto inserted = contracted_graph.insert_edge(ru, rv);
96-
contracted_edge_of_original[static_cast<std::size_t>(edge)] =
97-
static_cast<std::int64_t>(inserted);
128+
if (ru > rv) {
129+
std::swap(ru, rv);
130+
}
131+
survivors.push_back(Survivor{(ru << 32) | rv, edge});
98132
}
99133

134+
std::sort(
135+
survivors.begin(),
136+
survivors.end(),
137+
[](const Survivor &a, const Survivor &b) {
138+
return a.key < b.key;
139+
}
140+
);
141+
142+
std::vector<UndirectedGraph::Edge> contracted_edges;
143+
contracted_edges.reserve(survivors.size());
144+
145+
constexpr std::uint64_t no_key = std::numeric_limits<std::uint64_t>::max();
146+
std::uint64_t last_key = no_key;
147+
std::int64_t current_contracted = -1;
148+
for (const auto &survivor : survivors) {
149+
if (survivor.key != last_key) {
150+
const auto ru = survivor.key >> 32;
151+
const auto rv = survivor.key & std::uint64_t{0xFFFFFFFF};
152+
current_contracted = static_cast<std::int64_t>(contracted_edges.size());
153+
contracted_edges.push_back(UndirectedGraph::Edge{ru, rv});
154+
last_key = survivor.key;
155+
}
156+
contracted_edge_of_original[static_cast<std::size_t>(survivor.original_edge)] =
157+
current_contracted;
158+
}
159+
160+
auto contracted_graph = UndirectedGraph::from_sorted_unique_edges(
161+
number_of_components, std::move(contracted_edges), /*populate_lookup=*/false
162+
);
163+
100164
return AgreementContraction{
101165
std::move(contracted_graph),
102166
std::move(contracted_edge_of_original),

0 commit comments

Comments
 (0)