From e4d70f0e550edd77ee033fceb19ad857a79f4ffb Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Sun, 2 Aug 2026 11:20:31 +0200 Subject: [PATCH] image_preprocessing: inline the buffer accessors operator[], size(), data() and getBuffer() are one-line accessors that were defined in the .cpp. The build sets no link-time optimisation, so out of line each of them is a real call - once per pixel, from the CPU preprocessor, the CPU azimuthal integrator and the CPU spot finder - and they stop those loops vectorising at all. They show up in a profile directly: about six per cent of a whole azimuthal-integration-only run is spent in the call overhead of two accessors that do nothing but index a vector. Moving them into the header retires 30% fewer instructions on that run and takes the per-image CPU cost on a GPU-less pass from 34.6 to 24.2 ms, with the output bit for bit unchanged - same observation count, same cell, same merge statistics. It is worth nothing on the GPU path, where the image stays on the device, and everything on the paths that have no GPU to fall back on. This also explains a measurement that had been blamed on the pixel mask being a vector: a microbenchmark of that loop indexed a raw pointer and came out far faster than the same loop in the binary, and the difference was this call, not the mask. Measured properly the mask costs about 14% single-threaded rather than the 41% claimed, and at the thread counts this actually runs at the bit mask is FASTER than the byte mask it was proposed to become, because it moves eight times less traffic and the loop is bandwidth bound. That change should not be made. Co-Authored-By: Claude Opus 5 (1M context) --- .../ImagePreprocessorBuffer.cpp | 29 ------------------- .../ImagePreprocessorBuffer.h | 20 ++++++++----- 2 files changed, 12 insertions(+), 37 deletions(-) diff --git a/image_analysis/image_preprocessing/ImagePreprocessorBuffer.cpp b/image_analysis/image_preprocessing/ImagePreprocessorBuffer.cpp index abdb2c53..0a2728a3 100644 --- a/image_analysis/image_preprocessing/ImagePreprocessorBuffer.cpp +++ b/image_analysis/image_preprocessing/ImagePreprocessorBuffer.cpp @@ -5,35 +5,6 @@ ImagePreprocessorBuffer::ImagePreprocessorBuffer(size_t npixels) : buffer(npixels) {} -// Standard CPU operation -std::vector &ImagePreprocessorBuffer::getBuffer() { - return buffer; -} - -const std::vector &ImagePreprocessorBuffer::getBuffer() const { - return buffer; -} - -int32_t &ImagePreprocessorBuffer::operator[](size_t i) { - return buffer[i]; -} - -const int32_t &ImagePreprocessorBuffer::operator[](size_t i) const { - return buffer[i]; -} - -size_t ImagePreprocessorBuffer::size() const { - return buffer.size(); -} - -int32_t *ImagePreprocessorBuffer::data() { - return buffer.data(); -} - -const int32_t *ImagePreprocessorBuffer::data() const { - return buffer.data(); -} - void ImagePreprocessorBuffer::Gather(const std::vector &npixel, std::vector &values) const { values.resize(npixel.size()); for (size_t i = 0; i < npixel.size(); i++) diff --git a/image_analysis/image_preprocessing/ImagePreprocessorBuffer.h b/image_analysis/image_preprocessing/ImagePreprocessorBuffer.h index 3644ff03..a0ef88c5 100644 --- a/image_analysis/image_preprocessing/ImagePreprocessorBuffer.h +++ b/image_analysis/image_preprocessing/ImagePreprocessorBuffer.h @@ -14,15 +14,19 @@ public: explicit ImagePreprocessorBuffer(size_t npixels); virtual ~ImagePreprocessorBuffer() = default; - // Standard CPU operation - std::vector &getBuffer(); - const std::vector &getBuffer() const; - int32_t &operator[](size_t i); - const int32_t &operator[](size_t i) const; - size_t size() const; + // Standard CPU operation. Defined here rather than in the .cpp: these are called once per pixel by + // the CPU preprocessor, the CPU azimuthal integrator and the CPU spot finder, and the build has no + // link-time optimisation - so out of line they are a real call per pixel, and they stop the callers + // vectorising at all. Inlining them costs about a third of the per-image CPU work on every path + // that does not have a GPU, and leaves the results bit-identical. + std::vector &getBuffer() { return buffer; } + const std::vector &getBuffer() const { return buffer; } + int32_t &operator[](size_t i) { return buffer[i]; } + const int32_t &operator[](size_t i) const { return buffer[i]; } + size_t size() const { return buffer.size(); } - int32_t *data(); - const int32_t *data() const; + int32_t *data() { return buffer.data(); } + const int32_t *data() const { return buffer.data(); } // GPU operations (overriden in ImagePreprocessorBufferGPU virtual int32_t *getGPUBuffer();