minor C++ optimizations
Steven Parkes
This is mostly just inlining a couple of getter/setter pairs that are in the tightest loops. I can't prove that it's a significant improvement, but it doesn't hurt and it removes some distractions when I use callgrind.
It also includes doing some word size arithmetic at compile time which helps one of the getter/setter pairs. It's simpler than the dynamic method and gives the compiler a simpler basis for constant folding/propagation and strength reduction.
The only semantic change is making the retain() and release() methods of the reference counted objects non-virtual, which allows them to be inlined. None of the library code overrides these. In theory, someone could have derived classes that do … This stuff could all, in theory, go away someday to be replaced with standard library classes from C++11. Those don't provide the ability to override the functional equivalent.
diff --git a/cpp/core/src/zxing/common/BitArray.cpp b/cpp/core/src/zxing/common/BitArray.cpp
index 7229f70..92a19a8 100644
--- a/cpp/core/src/zxing/common/BitArray.cpp
+++ b/cpp/core/src/zxing/common/BitArray.cpp
@@ -59,14 +59,6 @@ size_t BitArray::getSize() {
return size_;
}
-bool BitArray::get(size_t i) {
- return (bits_[i >> logBits_] & (1 << (i & bitsMask_))) != 0;
-}
-
-void BitArray::set(size_t i) {
- bits_[i >> logBits_] |= 1 << (i & bitsMask_);
-}
-
void BitArray::setBulk(size_t i, unsigned int newBits) {
bits_[i >> logBits_] = newBits;
}
diff --git a/cpp/core/src/zxing/common/BitArray.h b/cpp/core/src/zxing/common/BitArray.h
index d8a041b..437ae50 100644
--- a/cpp/core/src/zxing/common/BitArray.h
+++ b/cpp/core/src/zxing/common/BitArray.h
@@ -42,8 +42,15 @@ public:
BitArray(size_t size);
~BitArray();
size_t getSize();
- bool get(size_t i);
- void set(size_t i);
+
+ bool get(size_t i) {
+ return (bits_[i >> logBits_] & (1 << (i & bitsMask_))) != 0;
+ }
+
+ void set(size_t i) {
+ bits_[i >> logBits_] |= 1 << (i & bitsMask_);
+ }
+
void setBulk(size_t i, unsigned int newBits);
void setRange(int start, int end);
void clear();
diff --git a/cpp/core/src/zxing/common/BitMatrix.cpp b/cpp/core/src/zxing/common/BitMatrix.cpp
index 326c05f..643c7a2 100644
--- a/cpp/core/src/zxing/common/BitMatrix.cpp
+++ b/cpp/core/src/zxing/common/BitMatrix.cpp
@@ -25,7 +25,6 @@
#include <sstream>
#include <string>
-using std::numeric_limits;
using std::ostream;
using std::ostringstream;
@@ -34,21 +33,9 @@ using zxing::BitArray;
using zxing::Ref;
namespace {
- unsigned int logDigits(unsigned digits) {
- unsigned log = 0;
- unsigned val = 1;
- while (val < digits) {
- log++;
- val <<= 1;
- }
- return log;
- }
-
- const unsigned int bitsPerWord = numeric_limits<unsigned int>::digits;
- const unsigned int logBits = logDigits(bitsPerWord);
- const unsigned int bitsMask = (1 << logBits) - 1;
-
- size_t wordsForSize(size_t width, size_t height) {
+ size_t wordsForSize(size_t width,
+ size_t height,
+ unsigned int logBits) {
size_t bits = width * height;
int arraySize = bits >> logBits;
if (bits - (arraySize << logBits) != 0) {
@@ -60,16 +47,14 @@ namespace {
BitMatrix::BitMatrix(size_t dimension) :
width_(dimension), height_(dimension), words_(0), bits_(NULL) {
-
- words_ = wordsForSize(width_, height_);
+ words_ = wordsForSize(width_, height_, logBits);
bits_ = new unsigned int[words_];
clear();
}
BitMatrix::BitMatrix(size_t width, size_t height) :
width_(width), height_(height), words_(0), bits_(NULL) {
-
- words_ = wordsForSize(width_, height_);
+ words_ = wordsForSize(width_, height_, logBits);
bits_ = new unsigned int[words_];
clear();
}
@@ -79,16 +64,6 @@ BitMatrix::~BitMatrix() {
}
-bool BitMatrix::get(size_t x, size_t y) const {
- size_t offset = x + width_ * y;
- return ((bits_[offset >> logBits] >> (offset & bitsMask)) & 0x01) != 0;
-}
-
-void BitMatrix::set(size_t x, size_t y) {
- size_t offset = x + width_ * y;
- bits_[offset >> logBits] |= 1 << (offset & bitsMask);
-}
-
void BitMatrix::flip(size_t x, size_t y) {
size_t offset = x + width_ * y;
bits_[offset >> logBits] ^= 1 << (offset & bitsMask);
@@ -135,7 +110,7 @@ Ref<BitArray> BitMatrix::getRow(int y, Ref<BitArray> row) {
size_t lastBit = i < lastWord ? bitsPerWord - 1 : end & bitsMask;
unsigned int mask;
if (firstBit == 0 && lastBit == logBits) {
- mask = numeric_limits<unsigned int>::max();
+ mask = std::numeric_limits<unsigned int>::max();
} else {
mask = 0;
for (size_t j = firstBit; j <= lastBit; j++) {
diff --git a/cpp/core/src/zxing/common/BitMatrix.h b/cpp/core/src/zxing/common/BitMatrix.h
index f734152..2a78a5a 100644
--- a/cpp/core/src/zxing/common/BitMatrix.h
+++ b/cpp/core/src/zxing/common/BitMatrix.h
@@ -34,14 +34,35 @@ private:
size_t words_;
unsigned int* bits_;
+#define ZX_LOG_DIGITS(digits) \
+ ((digits == 8) ? 3 : \
+ ((digits == 16) ? 4 : \
+ ((digits == 32) ? 5 : \
+ ((digits == 64) ? 6 : \
+ ((digits == 128) ? 7 : \
+ (-1))))))
+
+ static const unsigned int bitsPerWord =
+ std::numeric_limits<unsigned int>::digits;
+ static const unsigned int logBits = ZX_LOG_DIGITS(bitsPerWord);
+ static const unsigned int bitsMask = (1 << logBits) - 1;
+
public:
BitMatrix(size_t dimension);
BitMatrix(size_t width, size_t height);
~BitMatrix();
- // Inlining this does not really improve performance.
- bool get(size_t x, size_t y) const;
- void set(size_t x, size_t y);
+
+ bool get(size_t x, size_t y) const {
+ size_t offset = x + width_ * y;
+ return ((bits_[offset >> logBits] >> (offset & bitsMask)) & 0x01) != 0;
+ }
+
+ void set(size_t x, size_t y) {
+ size_t offset = x + width_ * y;
+ bits_[offset >> logBits] |= 1 << (offset & bitsMask);
+ }
+
void flip(size_t x, size_t y);
void clear();
void setRegion(size_t left, size_t top, size_t width, size_t height);
diff --git a/cpp/core/src/zxing/common/Counted.h b/cpp/core/src/zxing/common/Counted.h
index 306ed6c..dad79e3 100644
--- a/cpp/core/src/zxing/common/Counted.h
+++ b/cpp/core/src/zxing/common/Counted.h
@@ -21,7 +21,6 @@
*/
//#define DEBUG_COUNTING
-//using namespace std;
#include <iostream>
@@ -45,7 +44,7 @@ public:
}
virtual ~Counted() {
}
- virtual Counted *retain() {
+ Counted *retain() {
#ifdef DEBUG_COUNTING
cout << "retaining " << typeid(*this).name() << " " << this <<
" @ " << count_;
@@ -56,7 +55,7 @@ public:
#endif
return this;
}
- virtual void release() {
+ void release() {
#ifdef DEBUG_COUNTING
cout << "releasing " << typeid(*this).name() << " " << this <<
" @ " << count_;
diff --git a/cpp/core/src/zxing/common/HybridBinarizer.cpp b/cpp/core/src/zxing/common/HybridBinarizer.cpp
index 0a290f5..a6f7042 100644
--- a/cpp/core/src/zxing/common/HybridBinarizer.cpp
+++ b/cpp/core/src/zxing/common/HybridBinarizer.cpp
@@ -45,19 +45,22 @@ HybridBinarizer::~HybridBinarizer() {
}
-Ref<Binarizer> HybridBinarizer::createBinarizer(Ref<LuminanceSource> source) {
+Ref<Binarizer>
+HybridBinarizer::createBinarizer(Ref<LuminanceSource> source) {
return Ref<Binarizer> (new HybridBinarizer(source));
}
Ref<BitMatrix> HybridBinarizer::getBlackMatrix() {
- // Calculates the final BitMatrix once for all requests. This could be called once from the
- // constructor instead, but there are some advantages to doing it lazily, such as making
- // profiling easier, and not doing heavy lifting when callers don't expect it.
+ // Calculates the final BitMatrix once for all requests. This could
+ // be called once from the constructor instead, but there are some
+ // advantages to doing it lazily, such as making profiling easier,
+ // and not doing heavy lifting when callers don't expect it.
if (matrix_) {
return matrix_;
}
LuminanceSource& source = *getLuminanceSource();
- if (source.getWidth() >= MINIMUM_DIMENSION && source.getHeight() >= MINIMUM_DIMENSION) {
+ if (source.getWidth() >= MINIMUM_DIMENSION &&
+ source.getHeight() >= MINIMUM_DIMENSION) {
unsigned char* luminances = source.getMatrix();
int width = source.getWidth();
int height = source.getHeight();
@@ -69,14 +72,22 @@ Ref<BitMatrix> HybridBinarizer::getBlackMatrix() {
if ((height & BLOCK_SIZE_MASK) != 0) {
subHeight++;
}
- int* blackPoints = calculateBlackPoints(luminances, subWidth, subHeight, width, height);
+ int* blackPoints =
+ calculateBlackPoints(luminances, subWidth, subHeight, width, height);
Ref<BitMatrix> newMatrix (new BitMatrix(width, height));
- calculateThresholdForBlock(luminances, subWidth, subHeight, width, height, blackPoints, newMatrix);
+ calculateThresholdForBlock(luminances,
+ subWidth,
+ subHeight,
+ width,
+ height,
+ blackPoints,
+ newMatrix);
matrix_ = newMatrix;
- // N.B.: these deletes are inadequate if anything between the new and this point can throw.
- // As of this writing, it doesn't look like they do.
+ // N.B.: these deletes are inadequate if anything between the new
+ // and this point can throw. As of this writing, it doesn't look
+ // like they do.
delete [] blackPoints;
delete [] luminances;
@@ -87,8 +98,14 @@ Ref<BitMatrix> HybridBinarizer::getBlackMatrix() {
return matrix_;
}
-void HybridBinarizer::calculateThresholdForBlock(unsigned char* luminances, int subWidth, int subHeight,
- int width, int height, int blackPoints[], Ref<BitMatrix> matrix) {
+void
+HybridBinarizer::calculateThresholdForBlock(unsigned char* luminances,
+ int subWidth,
+ int subHeight,
+ int width,
+ int height,
+ int blackPoints[],
+ Ref<BitMatrix> const& matrix) {
for (int y = 0; y < subHeight; y++) {
int yoffset = y << BLOCK_SIZE_POWER;
if (yoffset + BLOCK_SIZE >= height) {
@@ -118,8 +135,12 @@ void HybridBinarizer::calculateThresholdForBlock(unsigned char* luminances, int
}
}
-void HybridBinarizer::threshold8x8Block(unsigned char* luminances, int xoffset, int yoffset, int threshold,
- int stride, Ref<BitMatrix> matrix) {
+void HybridBinarizer::threshold8x8Block(unsigned char* luminances,
+ int xoffset,
+ int yoffset,
+ int threshold,
+ int stride,
+ Ref<BitMatrix> const& matrix) {
for (int y = 0, offset = yoffset * stride + xoffset;
y < BLOCK_SIZE;
y++, offset += stride) {
diff --git a/cpp/core/src/zxing/common/HybridBinarizer.h b/cpp/core/src/zxing/common/HybridBinarizer.h
index 6c18f46..e7b7464 100644
--- a/cpp/core/src/zxing/common/HybridBinarizer.h
+++ b/cpp/core/src/zxing/common/HybridBinarizer.h
@@ -41,15 +41,28 @@ namespace zxing {
virtual Ref<BitMatrix> getBlackMatrix();
Ref<Binarizer> createBinarizer(Ref<LuminanceSource> source);
private:
- // We'll be using one-D arrays because C++ can't dynamically allocate 2D arrays
- int* calculateBlackPoints(unsigned char* luminances, int subWidth, int subHeight,
- int width, int height);
- void calculateThresholdForBlock(unsigned char* luminances, int subWidth, int subHeight,
- int width, int height, int blackPoints[], Ref<BitMatrix> matrix);
- void threshold8x8Block(unsigned char* luminances, int xoffset, int yoffset, int threshold,
- int stride, Ref<BitMatrix> matrix);
+ // We'll be using one-D arrays because C++ can't dynamically allocate 2D
+ // arrays
+ int* calculateBlackPoints(unsigned char* luminances,
+ int subWidth,
+ int subHeight,
+ int width,
+ int height);
+ void calculateThresholdForBlock(unsigned char* luminances,
+ int subWidth,
+ int subHeight,
+ int width,
+ int height,
+ int blackPoints[],
+ Ref<BitMatrix> const& matrix);
+ void threshold8x8Block(unsigned char* luminances,
+ int xoffset,
+ int yoffset,
+ int threshold,
+ int stride,
+ Ref<BitMatrix> const& matrix);
};
}
-#endif /* GLOBALHISTOGRAMBINARIZER_H_ */
+#endif