Skip to content

Commit e2aa8c8

Browse files
Edoardo Proserpioclaude
andcommitted
[PERF] Fix bugs, rule of five, and micro-optimizations
Bug fixes: - SiPMProperties: setDXtOff() was clearing m_HasXt instead of m_HasDXt - SiPMProperties: pdeSpectrum() returns const& (avoids map copy), pdeType() marked const - SiPMAnalogSignal: constructor takes wav by value so std::move() is not a no-op - SiPMAnalogSignal: integral()/peak() iterator off-by-one (skipped first sample, read past end) - SiPMAnalogSignal: toa() guard against i==0 underflow when start sample exceeds threshold - SiPMSensor: calculateSignalAmplitudes() uses hits[i-1]->time() for correct sequential recovery Rule of Five: - SiPMSmallVector: add copy/move constructor and assignment; compiler-generated shallow copy caused double-free on unordered_map rehash Micro-optimizations (RNG hot path): - Rand()/RandF(): generate raw integers directly into output vector buffer, eliminating sipmAlloc/sipmFree round-trip - randGaussian()/randGaussianF(): merge 3-loop Box-Muller into single pass, compute sqrtR once per pair Micro-optimizations (signal generation hot path): - generateSignal(): pre-extract times/amplitudes into flat local arrays to batch pointer-chasing cache misses - generateSignal(): add __builtin_prefetch for scattered SiPMHit* heap objects - generateSignal(): use __restrict__ on inner loop pointers to enable AVX2 FMA vectorization - addPhotons(): use assign() instead of copy-assignment to reuse existing vector capacity Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent e6c1914 commit e2aa8c8

6 files changed

Lines changed: 129 additions & 74 deletions

File tree

include/SiPMAnalogSignal.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class SiPMAnalogSignal {
3030
public:
3131
SiPMAnalogSignal() = default;
3232

33-
SiPMAnalogSignal(const std::vector<float>& wav, const double sampling) noexcept
33+
SiPMAnalogSignal(std::vector<float> wav, double sampling) noexcept
3434
: m_Waveform(std::move(wav)), m_Sampling(sampling) {};
3535

3636
float* data() noexcept { return m_Waveform.data(); }

include/SiPMProperties.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,10 @@ class SiPMProperties {
128128
constexpr double pde() const { return m_Pde; }
129129

130130
/// @brief Returns wavelength-PDE values if PdeType::kSpectrumPde is set
131-
std::map<double, double> pdeSpectrum() const { return m_PdeSpectrum; }
131+
const std::map<double, double>& pdeSpectrum() const { return m_PdeSpectrum; }
132132

133133
/// @brief Returns type of PDE calculation used.
134-
constexpr PdeType pdeType() { return m_HasPde; }
134+
constexpr PdeType pdeType() const { return m_HasPde; }
135135

136136
/// @brief Returns true if DCR is considered.
137137
constexpr bool hasDcr() const { return m_HasDcr; }
@@ -275,7 +275,7 @@ class SiPMProperties {
275275
/// @brief Turn off optical crosstalk
276276
constexpr void setXtOff() { m_HasXt = false; }
277277
/// @brief Turn off delayed optical crosstalk
278-
constexpr void setDXtOff() { m_HasXt = false; }
278+
constexpr void setDXtOff() { m_HasDXt = false; }
279279
/// @brief Turn off afterpulses
280280
constexpr void setApOff() { m_HasAp = false; }
281281
/// @brief Turns off slow component of the signal

include/SiPMTypes.h

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,66 @@ class SiPMSmallVector {
9696
}
9797
}
9898

99+
SiPMSmallVector(const SiPMSmallVector& other) noexcept
100+
: m_Size(other.m_Size), m_Capacity(other.m_Capacity) {
101+
if (other.isLocal()) {
102+
m_HeapStorage = nullptr;
103+
memcpy(m_LocalStorage, other.m_LocalStorage, m_Size * sizeof(T));
104+
} else {
105+
m_HeapStorage = static_cast<T*>(malloc(m_Capacity * sizeof(T)));
106+
memcpy(m_HeapStorage, other.m_HeapStorage, m_Size * sizeof(T));
107+
}
108+
}
109+
110+
SiPMSmallVector& operator=(const SiPMSmallVector& other) noexcept {
111+
if (this == &other) { return *this; }
112+
if (m_HeapStorage != nullptr) {
113+
free(m_HeapStorage);
114+
m_HeapStorage = nullptr;
115+
}
116+
m_Size = other.m_Size;
117+
m_Capacity = other.m_Capacity;
118+
if (other.isLocal()) {
119+
memcpy(m_LocalStorage, other.m_LocalStorage, m_Size * sizeof(T));
120+
} else {
121+
m_HeapStorage = static_cast<T*>(malloc(m_Capacity * sizeof(T)));
122+
memcpy(m_HeapStorage, other.m_HeapStorage, m_Size * sizeof(T));
123+
}
124+
return *this;
125+
}
126+
127+
SiPMSmallVector(SiPMSmallVector&& other) noexcept
128+
: m_Size(other.m_Size), m_Capacity(other.m_Capacity) {
129+
if (other.isLocal()) {
130+
m_HeapStorage = nullptr;
131+
memcpy(m_LocalStorage, other.m_LocalStorage, m_Size * sizeof(T));
132+
} else {
133+
m_HeapStorage = other.m_HeapStorage;
134+
other.m_HeapStorage = nullptr;
135+
other.m_Capacity = N;
136+
}
137+
other.m_Size = 0;
138+
}
139+
140+
SiPMSmallVector& operator=(SiPMSmallVector&& other) noexcept {
141+
if (this == &other) { return *this; }
142+
if (m_HeapStorage != nullptr) {
143+
free(m_HeapStorage);
144+
m_HeapStorage = nullptr;
145+
}
146+
m_Size = other.m_Size;
147+
m_Capacity = other.m_Capacity;
148+
if (other.isLocal()) {
149+
memcpy(m_LocalStorage, other.m_LocalStorage, m_Size * sizeof(T));
150+
} else {
151+
m_HeapStorage = other.m_HeapStorage;
152+
other.m_HeapStorage = nullptr;
153+
other.m_Capacity = N;
154+
}
155+
other.m_Size = 0;
156+
return *this;
157+
}
158+
99159
void push_back(const T& value) {
100160
if (m_Size == m_Capacity ) {
101161
if (isLocal()) {

src/SiPMAnalogSignal.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ double SiPMAnalogSignal::integral(const double intstart, const double intgate, c
1818
const auto end = m_Waveform.cbegin() + (intstart + intgate) / m_Sampling;
1919
bool isOver = false;
2020
float integral = 0;
21-
while (start++ < end) {
21+
while (start < end) {
2222
if (*start > threshold) {
2323
isOver = true;
2424
}
25-
integral += *start;
25+
integral += *start++;
2626
}
2727

2828
return isOver ? integral * m_Sampling : -1;
@@ -40,9 +40,9 @@ double SiPMAnalogSignal::peak(const double intstart, const double intgate, const
4040
auto start = m_Waveform.begin() + intstart / m_Sampling;
4141
const auto end = m_Waveform.cbegin() + (intstart + intgate) / m_Sampling;
4242
float peak = -1;
43-
while (start++ < end) {
43+
while (start < end) {
4444
if (*start > threshold && *start > peak) {
45-
peak = *start;
45+
peak = *start++;
4646
}
4747
}
4848

@@ -82,7 +82,9 @@ double SiPMAnalogSignal::toa(const double intstart, const double intgate, const
8282
const uint32_t start = intstart / m_Sampling;
8383
const uint32_t end = (intstart + intgate) / m_Sampling;
8484

85-
for (uint32_t i = start; i < end - 1; ++i) {
85+
if(m_Waveform[start] > threshold){ return start; }
86+
87+
for (uint32_t i = start; i < end; ++i) {
8688
if (m_Waveform[i] > threshold) {
8789
const float d = (threshold - m_Waveform[i - 1]) / (m_Waveform[i] - m_Waveform[i - 1]);
8890
return (i - start - 1 + d) * m_Sampling;

src/SiPMRandom.cpp

Lines changed: 34 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -322,15 +322,15 @@ pair<uint32_t> SiPMRandom::randInteger2(const uint32_t max) noexcept {
322322
* @param n Number of values to generate
323323
*/
324324
std::vector<double> SiPMRandom::Rand(const uint32_t n) {
325-
// Integer multiple of 64 grater than n*sizeof(double)
326325
std::vector<double> out(n);
327-
328-
uint64_t* u64 = (uint64_t*)sipmAlloc(sizeof(uint64_t) * n);
326+
// Generate raw u64 directly into the output buffer (sizeof(uint64_t)==sizeof(double)),
327+
// then convert each element in-place before reading it as a double.
328+
auto* const u64 = reinterpret_cast<uint64_t*>(out.data());
329329
m_rng.getRand(u64, n);
330330
for (uint32_t i = 0; i < n; ++i) {
331-
out[i] = (u64[i] >> 11) * 0x1p-53;
331+
const uint64_t u = u64[i];
332+
out[i] = (u >> 11) * 0x1p-53;
332333
}
333-
sipmFree(u64);
334334
return out;
335335
}
336336

@@ -339,15 +339,14 @@ std::vector<double> SiPMRandom::Rand(const uint32_t n) {
339339
*/
340340
std::vector<float> SiPMRandom::RandF(const uint32_t n) {
341341
std::vector<float> out(n);
342-
343-
// simd8 allocates data
344-
uint32_t* u32 = (uint32_t*)sipmAlloc(sizeof(float) * n);
342+
// Generate raw u32 directly into the output buffer (sizeof(uint32_t)==sizeof(float)),
343+
// then convert each element in-place before reading it as a float.
344+
auto* const u32 = reinterpret_cast<uint32_t*>(out.data());
345345
m_rng.getRand(u32, n);
346346
for (uint32_t i = 0; i < n; ++i) {
347-
// See SiPMRandom.h for details
348-
out[i] = (u32[i] >> 8) * 0x1p-24f;
347+
const uint32_t u = u32[i];
348+
out[i] = (u >> 8) * 0x1p-24f;
349349
}
350-
sipmFree(u32);
351350
return out;
352351
}
353352

@@ -360,30 +359,23 @@ std::vector<double> SiPMRandom::randGaussian(const double mu, const double sigma
360359
std::vector<double> out(n);
361360
const std::vector<double> u = Rand(n);
362361
constexpr double TWO_PI = 2 * M_PI;
363-
double* r = (double*)sipmAlloc(sizeof(double) * n);
364362

365-
for (uint32_t i = 0; i < n - 1; i += 2) {
366-
const double R = -2 * log(u[i]);
367-
r[i] = R;
368-
r[i + 1] = R;
369-
}
363+
// Single pass: compute sqrtR once per pair, apply to both sin and cos outputs.
364+
const uint32_t pairs = n & ~1u;
365+
for (uint32_t i = 0; i < pairs; i += 2) {
366+
const double sqrtR = sqrt(-2.0 * log(u[i]));
367+
double s, c;
370368
#ifdef __APPLE__
371-
for (uint32_t i = 0; i < n - 1; i += 2) {
372-
double* ptr = out.data() + i;
373-
__sincos(TWO_PI * u[i + 1], ptr, ptr + 1);
374-
}
369+
__sincos(TWO_PI * u[i + 1], &s, &c);
375370
#else
376-
for (uint32_t i = 0; i < n - 1; i += 2) {
377-
double* ptr = out.data() + i;
378-
sincos(TWO_PI * u[i + 1], ptr, ptr + 1);
379-
}
371+
sincos(TWO_PI * u[i + 1], &s, &c);
380372
#endif
381-
for (uint32_t i = 0; i < n; ++i) {
382-
out[i] = out[i] * sqrt(r[i]) * sigma + mu;
373+
out[i] = s * sqrtR * sigma + mu;
374+
out[i + 1] = c * sqrtR * sigma + mu;
375+
}
376+
if (n & 1u) {
377+
out[n - 1] = randGaussian(mu, sigma);
383378
}
384-
385-
out[n - 1] = randGaussian(mu, sigma);
386-
sipmFree(r);
387379
return out;
388380
}
389381

@@ -396,30 +388,23 @@ std::vector<float> SiPMRandom::randGaussianF(const float mu, const float sigma,
396388
std::vector<float> out(n);
397389
const std::vector<float> u = RandF(n);
398390
constexpr float TWO_PI = 2 * M_PI;
399-
float* r = (float*)sipmAlloc(sizeof(float) * n);
400391

401-
for (uint32_t i = 0; i < n - 1; i += 2) {
402-
const float R = -2 * logf(u[i]);
403-
r[i] = R;
404-
r[i + 1] = R;
405-
}
392+
// Single pass: compute sqrtR once per pair, apply to both sin and cos outputs.
393+
const uint32_t pairs = n & ~1u;
394+
for (uint32_t i = 0; i < pairs; i += 2) {
395+
const float sqrtR = sqrtf(-2.0f * logf(u[i]));
396+
float s, c;
406397
#ifdef __APPLE__
407-
for (uint32_t i = 0; i < n - 1; i += 2) {
408-
float* ptr = out.data() + i;
409-
__sincosf(TWO_PI * u[i + 1], ptr, ptr + 1);
410-
}
398+
__sincosf(TWO_PI * u[i + 1], &s, &c);
411399
#else
412-
for (uint32_t i = 0; i < n - 1; i += 2) {
413-
float* ptr = out.data() + i;
414-
sincosf(TWO_PI * u[i + 1], ptr, ptr + 1);
415-
}
400+
sincosf(TWO_PI * u[i + 1], &s, &c);
416401
#endif
417-
for (uint32_t i = 0; i < n; ++i) {
418-
out[i] = out[i] * sqrtf(r[i]) * sigma + mu;
402+
out[i] = s * sqrtR * sigma + mu;
403+
out[i + 1] = c * sqrtR * sigma + mu;
404+
}
405+
if (n & 1u) {
406+
out[n - 1] = randGaussianF(mu, sigma);
419407
}
420-
421-
out[n - 1] = randGaussianF(mu, sigma);
422-
sipmFree(r);
423408
return out;
424409
}
425410

src/SiPMSensor.cpp

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,18 +38,20 @@ void SiPMSensor::addPhoton(const double val1, const double val2) {
3838
}
3939

4040
void SiPMSensor::addPhotons(const std::vector<double>& val) {
41-
m_PhotonTimes = val;
41+
m_PhotonTimes.assign(val.begin(), val.end());
4242
m_PhotonWavelengths.clear();
4343
}
4444

4545
void SiPMSensor::addPhotons(const std::vector<double>& val1, const std::vector<double>& val2) {
46-
m_PhotonTimes = val1;
47-
m_PhotonWavelengths = val2;
46+
m_PhotonTimes.assign(val1.begin(), val1.end());
47+
m_PhotonWavelengths.assign(val2.begin(), val2.end());
4848
}
4949

5050
void SiPMSensor::runEvent() {
51-
const std::vector<float> waveform = m_rng.randGaussianF(0.0, m_Properties.snrLinear(), m_Properties.nSignalPoints());
52-
m_Signal = SiPMAnalogSignal(waveform, m_Properties.sampling());
51+
m_Signal = SiPMAnalogSignal(
52+
m_rng.randGaussianF(0.0, m_Properties.snrLinear(), m_Properties.nSignalPoints()),
53+
m_Properties.sampling()
54+
);
5355
addDcrEvents();
5456

5557
addPhotoelectrons();
@@ -359,7 +361,7 @@ void SiPMSensor::calculateSignalAmplitudes() {
359361
std::sort(hits.begin(), hits.end(), [](const SiPMHit* a, const SiPMHit* b) { return *a < *b; });
360362
// Calculate amplitude
361363
for (uint32_t i = 1; i < n_hits; ++i) {
362-
const double delay = hits[i]->time() - hits[0]->time();
364+
const double delay = hits[i]->time() - hits[i-1]->time();
363365
hits[i]->amplitude() *= 1 - exp(-delay * recoveryRate);
364366
}
365367
}
@@ -368,24 +370,30 @@ void SiPMSensor::calculateSignalAmplitudes() {
368370

369371
void SiPMSensor::generateSignal() {
370372
const uint32_t nSignalPoints = m_Properties.nSignalPoints();
371-
// Reciprocal of sampling (avoid division later)
372-
const float recSampling = 1 / m_Properties.sampling();
373+
const float recSampling = 1.0f / m_Properties.sampling();
373374

374-
// Convert ns in units of samples and round to nearest
375+
// Pre-extract hit data into flat local arrays to eliminate pointer chasing
376+
// through individually heap-allocated SiPMHit objects in the accumulation loop.
375377
std::vector<uint32_t> times(m_nTotalHits);
378+
std::vector<float> amplitudes(m_nTotalHits);
376379
for (uint32_t i = 0; i < m_nTotalHits; ++i) {
377-
times[i] = std::floor(m_Hits[i]->time() * recSampling);
380+
if (i + 2 < m_nTotalHits) {
381+
__builtin_prefetch(m_Hits[i + 2], 0, 0);
382+
}
383+
times[i] = static_cast<uint32_t>(std::floor(m_Hits[i]->time() * recSampling));
384+
amplitudes[i] = m_Hits[i]->amplitude();
378385
}
379386

380-
// This loop should be vectorized and unrolled by compiler
381387
for (uint32_t i = 0; i < m_nTotalHits; ++i) {
382388
const uint32_t time = times[i];
383-
const float amplitude = m_Hits[i]->amplitude();
384-
385-
389+
if (time >= nSignalPoints) { continue; }
390+
const float amplitude = amplitudes[i];
386391
const uint32_t endPoint = nSignalPoints - time;
387-
float* signalPtr = m_Signal.data() + time;
388-
const float* signalShapePtr = m_SignalShape.data();
392+
393+
// __restrict__ proves no aliasing between signal and shape buffers,
394+
// enabling the compiler to emit vectorized FMA for this inner loop.
395+
float* __restrict__ signalPtr = m_Signal.data() + time;
396+
const float* __restrict__ signalShapePtr = m_SignalShape.data();
389397

390398
for (uint32_t j = 0; j < endPoint; ++j) {
391399
signalPtr[j] += signalShapePtr[j] * amplitude;

0 commit comments

Comments
 (0)