Skip to content

Commit 907f3ce

Browse files
committed
[RF] Avoid double-virtual funciton call in RooBinning
In `RooBinning`, the virtual `binNumber()` function was implemented by calling the virtual `rawBinNumber()` function. The overhead from the virtual function call is avoided by using an inline implementation function instead. Furthermore, the `rawBinNumber()` function was removed from the RooAbsBinning interface, because it makes only sense in the context of the RooBinning implementation. Also, the functionality to look up bin boundaries by value in `RooBinning` is removed, as such floating point comparisons should not be done.
1 parent 0aff417 commit 907f3ce

File tree

3 files changed

+17
-20
lines changed

3 files changed

+17
-20
lines changed

roofit/roofitcore/inc/RooAbsBinning.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ class RooAbsBinning : public TNamed, public RooPrintable {
3939
}
4040
virtual Int_t numBoundaries() const = 0 ;
4141
virtual Int_t binNumber(double x) const = 0 ;
42-
virtual Int_t rawBinNumber(double x) const { return binNumber(x) ; }
4342
virtual double binCenter(Int_t bin) const = 0 ;
4443
virtual double binWidth(Int_t bin) const = 0 ;
4544
virtual double binLow(Int_t bin) const = 0 ;

roofit/roofitcore/inc/RooBinning.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class RooBinning : public RooAbsBinning {
3939
return _nbins+1;
4040
}
4141
Int_t binNumber(double x) const override;
42-
Int_t rawBinNumber(double x) const override;
42+
Int_t rawBinNumber(double x) const;
4343
virtual double nearestBoundary(double x) const;
4444

4545
void setRange(double xlo, double xhi) override;
@@ -70,8 +70,6 @@ class RooBinning : public RooAbsBinning {
7070
void addUniform(Int_t nBins, double xlo, double xhi);
7171
bool removeBoundary(double boundary);
7272

73-
bool hasBoundary(double boundary);
74-
7573
protected:
7674

7775
bool binEdges(Int_t bin, double& xlo, double& xhi) const;

roofit/roofitcore/src/RooBinning.cxx

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -157,14 +157,6 @@ bool RooBinning::removeBoundary(double boundary)
157157
return true;
158158
}
159159

160-
////////////////////////////////////////////////////////////////////////////////
161-
/// Check if boundary exists at given value
162-
163-
bool RooBinning::hasBoundary(double boundary)
164-
{
165-
return std::binary_search(_boundaries.begin(), _boundaries.end(), boundary);
166-
}
167-
168160
////////////////////////////////////////////////////////////////////////////////
169161
/// Add array of nbins uniformly sized bins in range [xlo,xhi]
170162

@@ -176,14 +168,26 @@ void RooBinning::addUniform(Int_t nbins, double xlo, double xhi)
176168
(double(i) / double(nbins)) * xhi);
177169
}
178170

171+
namespace {
172+
173+
inline int rawBinNumberImpl(double x, std::vector<double> const& boundaries) {
174+
auto it = std::lower_bound(boundaries.begin(), boundaries.end(), x);
175+
// always return valid bin number
176+
while (boundaries.begin() != it &&
177+
(boundaries.end() == it || boundaries.end() == it + 1 || x < *it)) --it;
178+
return it - boundaries.begin();
179+
}
180+
181+
}
182+
179183
////////////////////////////////////////////////////////////////////////////////
180184
/// Return sequential bin number that contains value x where bin
181185
/// zero is the first bin with an upper boundary above the lower bound
182186
/// of the range
183187

184188
Int_t RooBinning::binNumber(double x) const
185189
{
186-
return std::max(0, std::min(_nbins, rawBinNumber(x) - _blo));
190+
return std::max(0, std::min(_nbins, rawBinNumberImpl(x, _boundaries) - _blo));
187191
}
188192

189193
////////////////////////////////////////////////////////////////////////////////
@@ -193,14 +197,10 @@ Int_t RooBinning::binNumber(double x) const
193197

194198
Int_t RooBinning::rawBinNumber(double x) const
195199
{
196-
std::vector<double>::const_iterator it = std::lower_bound(
197-
_boundaries.begin(), _boundaries.end(), x);
198-
// always return valid bin number
199-
while (_boundaries.begin() != it &&
200-
(_boundaries.end() == it || _boundaries.end() == it + 1 || x < *it)) --it;
201-
return it - _boundaries.begin();
200+
return rawBinNumberImpl(x, _boundaries);
202201
}
203202

203+
204204
////////////////////////////////////////////////////////////////////////////////
205205
/// Return the value of the nearest boundary to x
206206

@@ -254,7 +254,7 @@ void RooBinning::updateBinCount()
254254
_nbins = -1;
255255
return;
256256
}
257-
_blo = rawBinNumber(_xlo);
257+
_blo = rawBinNumberImpl(_xlo, _boundaries);
258258
std::vector<double>::const_iterator it = std::lower_bound(
259259
_boundaries.begin(), _boundaries.end(), _xhi);
260260
if (_boundaries.begin() != it && (_boundaries.end() == it || _xhi < *it)) --it;

0 commit comments

Comments
 (0)