Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

C++ abstraction layer: Types that are backed by memory handles now ha…

…ve copy constructors that perform deep copies if the original object is backed by transient memory. See also the updated documentation of AbstractHandle.h. [MADLIB-253]
  • Loading branch information...
commit c47b9b30c2d14544125b1b2e5f0607c56c25b80e 1 parent 33273a6
Florian Schoppmann authored
View
7 src/CMakeLists.txt
@@ -330,8 +330,13 @@ if(APPLE)
message(STATUS "Will build madlib for same architecture as detected in "
"${ACCELERATE_FRAMEWORK}, which has architectures "
"${ACCELERATE_ARCHS}")
+ # FIXME: __ZN6madlib4dbal16defaultAllocatorEv is the mangled name of
+ # madlib::dbal::defaultAllocator().
+ # A better way would be do find out the mangled name at compile time, even
+ # better yet this option should not be necessary.
set_target_properties(madlib PROPERTIES
- OSX_ARCHITECTURES "${ACCELERATE_ARCHS}")
+ OSX_ARCHITECTURES "${ACCELERATE_ARCHS}"
+ LINK_FLAGS "-Wl,-U,__ZN6madlib4dbal16defaultAllocatorEv")
# On the Mac, the Accelerate framework is already an umbrella for everything
# we need for Armadillo
View
7 src/dbal/AbstractAllocator.hpp
@@ -55,3 +55,10 @@ class AbstractAllocator {
*/
virtual void free(void *inPtr) const throw() = 0;
};
+
+/**
+ * @brief Return the default allocator
+ *
+ * This needs to be defined in the DBMS-specific part.
+ */
+AbstractAllocator &defaultAllocator();
View
57 src/dbal/AbstractHandle.hpp
@@ -17,7 +17,8 @@
* this abstract base class.
*
* In a handle's destructor, all memory blocks owned by the handle must be
- * deallocated. A handle is said to own a memory block, if
+ * deallocated. A handle is said to own a memory block (indicated by
+ * <tt>mMemoryController == kSelf</tt>), if:
* -# One of the following two condition holds:
* - It allocated the memory block itself.
* - The memory block was allocated during the clone() operation that
@@ -30,6 +31,33 @@
*/
class AbstractHandle {
public:
+ /**
+ * @brief Options for memory ownership
+ *
+ * - \c kGlobal indicates that the caller forsees that the memory
+ * block(s) pointed to by this handle will never go of scope while this
+ * memory handle is alive. Unless a distinct (mutable) copy of the memory
+ * block(s) is needed, it is gemerally preferable to copy shared pointers
+ * to this handle (i.e., make sa shallow copy of only the reference) than
+ * to do a clone (i.e., make a deep copy).
+ * - \c kLocal indicates that the memory block pointed to by this memory
+ * handle is transient. There is no guarantee that the memory block(s)
+ * pointed to by this handle remain valid while this handle is alive.
+ * (For instance, this would be the case if the memory block is allocated
+ * on the stack.)
+ * Copying an object backed by this memory handle (e.g., an array or a
+ * matrix) typically involves a clone(), which creates a copy of the
+ * memory block(s) pointed to by this handle. The cloned memory handle
+ * will then own that memory block(s) and
+ * have \c mMemoryController set to \c kSelf.
+ * - \c kSelf indicates that this memory handle owns its memory block(s) and
+ * will also take care of deallocating them at the end of its lifetime.
+ */
+ enum MemoryController { kGlobal, kLocal, kSelf };
+
+ AbstractHandle(MemoryController inCtrl)
+ : mMemoryController(inCtrl) { }
+
virtual ~AbstractHandle() { }
/**
@@ -38,12 +66,33 @@ class AbstractHandle {
virtual void *ptr() = 0;
/**
- * @brief Relinquish ownership of the memory blocks owned by this handle
+ * @brief Relinquish ownership of the memory blocks owned by this handle.
+ *
+ * This sets mMemoryController to kLocal.
*/
- virtual void release() = 0;
+ virtual void release() {
+ if (mMemoryController == kSelf)
+ mMemoryController = kLocal;
+ };
/**
- * @brief Return a deep (and fully independent) copy of this memory handle.
+ * @brief Return a deep (and fully independent) copy of this memory handle
*/
virtual MemHandleSPtr clone() const = 0;
+
+ /**
+ * @brief Return ownership of the memory block(s) pointed to by this memory
+ * handle
+ */
+ MemoryController memoryController() const {
+ return mMemoryController;
+ }
+
+ static inline MemHandleSPtr cloneIfNotGlobal(const MemHandleSPtr &inHandle) {
+ return inHandle->mMemoryController == kGlobal ?
+ inHandle : inHandle->clone();
+ }
+
+private:
+ MemoryController mMemoryController;
};
View
2  src/dbal/AnyType_proto.hpp
@@ -7,7 +7,7 @@
/**
* @brief Class for representing any type.
*
- * Instances of this class act a proxy for any arbitrary ConcreteType<T>.
+ * Instances of this class act a proxy for any arbitrary \ref ConcreteType.
*/
class AnyType : public AbstractType {
// ConcreteType<AnyTypeVector>::getValueByID() accesses mDelegate, so we
View
20 src/dbal/Array.hpp
@@ -26,9 +26,8 @@ class Array : public multi_array_ref<T, NumDims> {
inline Array(
const Array<T, NumDims> &inArray)
- : multi_array_ref<T, NumDims>(
- inArray),
- mMemoryHandle(inArray.mMemoryHandle)
+ : multi_array_ref<T, NumDims>(inArray),
+ mMemoryHandle(AbstractHandle::cloneIfNotGlobal(inArray.mMemoryHandle))
{ }
inline Array(
@@ -75,21 +74,6 @@ class Array : public multi_array_ref<T, NumDims> {
return *this;
}
- // FIXME: We might "own" the cloned memory handle, so we should release it.
- // This is not a problem for PostgreSQL, but we need to implement reference
- // counting. Either based on shared_ptr or our own.
-/* inline Array(
- const Array_const<T, NumDims> &inArray)
- : multi_array_ref<T, NumDims>(
- NULL,
- utils::shapeToExtents<NumDims>(inArray.shape())),
- mMemoryHandle(
- inArray.memoryHandle().clone()) {
-
- this->set_base_ptr(mMemoryHandle->ptr());
- }
-*/
-
/**
* @brief Rebind the array to a different chunk of memory (referred to by a
* memory handle)
View
2  src/dbal/Array_const.hpp
@@ -29,7 +29,7 @@ class Array_const : public const_multi_array_ref<T, NumDims> {
const Array_const<T, NumDims> &inArray)
: const_multi_array_ref<T, NumDims>(
inArray),
- mMemoryHandle(inArray.mMemoryHandle)
+ mMemoryHandle(AbstractHandle::cloneIfNotGlobal(inArray.mMemoryHandle))
{ }
inline Array_const(
View
14 src/dbal/Matrix.hpp
@@ -45,10 +45,7 @@ class Matrix : public arma::Mat<eT> {
inNumRows * inNumCols,
static_cast<eT*>(NULL) /* pure type parameter */)) {
- using arma::access;
- using arma::Mat;
-
- access::rw(Mat<eT>::mem) = mMemoryHandle->ptr();
+ arma::access::rw(arma::Mat<eT>::mem) = mMemoryHandle->ptr();
}
inline Matrix(
@@ -67,13 +64,16 @@ class Matrix : public arma::Mat<eT> {
inline Matrix(
const Matrix<eT> &inMat)
: arma::Mat<eT>(
- const_cast<eT*>(inMat.memptr()),
+ NULL,
inMat.n_rows,
inMat.n_cols,
false /* copy_aux_mem */,
true /* strict */),
- mMemoryHandle(inMat.mMemoryHandle)
- { }
+ mMemoryHandle(AbstractHandle::cloneIfNotGlobal(inMat.mMemoryHandle)) {
+
+ arma::access::rw(arma::Mat<eT>::mem) = static_cast<eT*>(
+ mMemoryHandle->ptr());
+ }
template<typename T1>
inline const Matrix &operator=(const arma::Base<eT,T1>& X) {
View
51 src/dbal/TransparentHandle.hpp
@@ -15,27 +15,54 @@
*/
class TransparentHandle : public AbstractHandle {
public:
- static MemHandleSPtr create(void *inPtr) {
- return MemHandleSPtr(new TransparentHandle(inPtr));
- }
+ using AbstractHandle::MemoryController;
- void *ptr() {
- return mPtr;
+ virtual ~TransparentHandle() {
+ if (memoryController() == kSelf)
+ defaultAllocator().free(mPtr);
}
- void release() { };
-
/**
- * @brief Report an error because a TransparentHandle cannot be cloned.
+ * @brief Return a shared pointer to a new TransparentHandle
+ *
+ * @param inPtr Memory block to point to, or to copy if
+ * <tt>inCtrl == kSelf</tt>
+ * @param inSize Size of the memory block. 0 if unknown.
+ * @param inCtrl Ownership of the memory block
*/
+ static inline MemHandleSPtr create(void *inPtr, size_t inSize = 0,
+ MemoryController inCtrl = kLocal) {
+
+ return MemHandleSPtr(new TransparentHandle(inPtr, inSize, inCtrl));
+ }
+
+ inline void *ptr() {
+ return mPtr;
+ }
+
MemHandleSPtr clone() const {
- throw std::logic_error("Internal error: TransparentHandle::clone() "
- "called.");
+ if (mSize == 0)
+ throw std::logic_error("Internal error: TransparentHandle::clone() "
+ "called on a handle with unknown size.");
+
+ return MemHandleSPtr( new TransparentHandle(mPtr, mSize, kSelf) );
}
protected:
- TransparentHandle(void *inPtr)
- : mPtr(inPtr) { }
+ TransparentHandle(void *inPtr, size_t inSize, MemoryController inCtrl)
+ : AbstractHandle(inCtrl), mPtr(inPtr), mSize(inSize) {
+
+ if (inCtrl == kSelf) {
+ mPtr = defaultAllocator().allocate(inSize);
+ std::memcpy(mPtr, inPtr, inSize);
+ }
+ }
void *mPtr;
+
+ /**
+ * @brief Size of the memory block. A size of zero indicates that the size
+ * is unknown.
+ */
+ size_t mSize;
};
View
26 src/dbal/Vector.hpp
@@ -43,10 +43,8 @@ class Vector : public T<eT> {
inNumElem,
static_cast<eT*>(NULL) /* pure type parameter */)) {
- using arma::access;
- using arma::Mat;
-
- access::rw(Mat<eT>::mem) = static_cast<eT*>(mMemoryHandle->ptr());
+ arma::access::rw(arma::Mat<eT>::mem) =
+ static_cast<eT*>(mMemoryHandle->ptr());
}
inline Vector(
@@ -63,22 +61,30 @@ class Vector : public T<eT> {
inline Vector(
const Vector<T, eT> &inVec)
: T<eT>(
- const_cast<eT*>(inVec.memptr()),
+ NULL,
inVec.n_elem,
false /* copy_aux_mem */,
true /* strict */),
- mMemoryHandle(inVec.mMemoryHandle)
- { }
+ mMemoryHandle(
+ AbstractHandle::cloneIfNotGlobal(inVec.mMemoryHandle)) {
+
+ arma::access::rw(arma::Mat<eT>::mem) =
+ static_cast<eT*>(mMemoryHandle->ptr());
+ }
inline Vector(
const Array<eT> &inArray)
: T<eT>(
- const_cast<eT*>(inArray.data()),
+ NULL,
inArray.size(),
false /* copy_aux_mem */,
true /* strict */),
- mMemoryHandle(inArray.memoryHandle())
- { }
+ mMemoryHandle(
+ AbstractHandle::cloneIfNotGlobal(inArray.memoryHandle())) {
+
+ arma::access::rw(arma::Mat<eT>::mem) =
+ static_cast<eT*>(mMemoryHandle->ptr());
+ }
template<typename T1>
inline const Vector &operator=(const arma::Base<eT,T1>& X) {
View
30 src/dbal/Vector_const.hpp
@@ -44,7 +44,7 @@ class Vector_const : public arma::Base< eT, Vector_const<T, eT> > {
const uint32_t inNumElem)
: mMemoryHandle(inHandle),
mVector(
- static_cast<eT*>(inHandle->ptr()),
+ static_cast<eT*>(mMemoryHandle->ptr()),
inNumElem,
false /* copy_aux_mem */,
true /* strict */),
@@ -55,9 +55,10 @@ class Vector_const : public arma::Base< eT, Vector_const<T, eT> > {
inline Vector_const(
const Vector<T, eT> &inVec)
- : mMemoryHandle(inVec.mMemoryHandle),
+ : mMemoryHandle(
+ AbstractHandle::cloneIfNotGlobal(inVec.mMemoryHandle)),
mVector(
- const_cast<eT*>(inVec.memptr()),
+ const_cast<eT*>(mMemoryHandle->ptr()),
inVec.n_elem,
false /* copy_aux_mem */,
true /* strict */),
@@ -68,9 +69,10 @@ class Vector_const : public arma::Base< eT, Vector_const<T, eT> > {
inline Vector_const(
const Array<eT> &inArray)
- : mMemoryHandle(inArray.memoryHandle()),
+ : mMemoryHandle(
+ AbstractHandle::cloneIfNotGlobal(inArray.memoryHandle())),
mVector(
- const_cast<eT*>(inArray.data()),
+ static_cast<eT*>(mMemoryHandle->ptr()),
inArray.size(),
false /* copy_aux_mem */,
true /* strict */),
@@ -78,21 +80,13 @@ class Vector_const : public arma::Base< eT, Vector_const<T, eT> > {
n_cols(mVector.n_cols),
n_elem(mVector.n_elem)
{ }
-
- /**
- * @internal
- * This constructor will only be generated if \c T is a subclass of
- * \c const_multi_array_ref. This applies to Array<eT> and Array_const<eT>.
- */
- template <typename ArrayType>
- inline Vector_const(const ArrayType &inArray,
- typename enable_if<
- boost::is_base_of< const_multi_array_ref<eT, 1>, ArrayType >
- >::type * /* dummy */ = NULL)
- : mMemoryHandle(inArray.memoryHandle()),
+ inline Vector_const(
+ const Array_const<eT> &inArray)
+ : mMemoryHandle(
+ AbstractHandle::cloneIfNotGlobal(inArray.memoryHandle())),
mVector(
- const_cast<eT*>(inArray.data()),
+ static_cast<eT*>(mMemoryHandle->ptr()),
inArray.size(),
false /* copy_aux_mem */,
true /* strict */),
View
32 src/modules/regress/linear.cpp
@@ -67,8 +67,11 @@ class LinearRegression::TransitionState {
/**
* @brief Initialize the transition state. Only called for first row.
*
- * @param inAllocator Will be used to allocate the memory for the
- * transition state. Must fill the memory block with zeros.
+ * @param inAllocator Allocator for the memory transition state. Must fill
+ * the memory block with zeros.
+ * @param inWidthOfX Number of independent variables. The first row of data
+ * determines the size of the transition state. This size is a quadratic
+ * function of inWidthOfX.
*/
inline void initialize(AllocatorSPtr inAllocator,
const uint16_t inWidthOfX) {
@@ -98,6 +101,16 @@ class LinearRegression::TransitionState {
}
/**
+ * @brief Who should own TransparentHandles pointing to slices of mStorage?
+ */
+ AbstractHandle::MemoryController memoryController() const {
+ AbstractHandle::MemoryController ctrl =
+ mStorage.memoryHandle()->memoryController();
+
+ return (ctrl == AbstractHandle::kSelf ? AbstractHandle::kLocal : ctrl);
+ }
+
+ /**
* @brief Rebind to a new storage array
*
* @param inWidthOfX If this value is positive, use it as the number of
@@ -109,17 +122,20 @@ class LinearRegression::TransitionState {
numRows.rebind(&mStorage[0]);
widthOfX.rebind(&mStorage[1]);
- if (inWidthOfX == 0)
- inWidthOfX = widthOfX;
+ if (inWidthOfX != 0)
+ widthOfX = inWidthOfX;
y_sum.rebind(&mStorage[2]);
y_square_sum.rebind(&mStorage[3]);
X_transp_Y.rebind(
- TransparentHandle::create(&mStorage[4]),
- inWidthOfX);
+ TransparentHandle::create(&mStorage[4], widthOfX * sizeof(double),
+ memoryController()),
+ widthOfX);
X_transp_X.rebind(
- TransparentHandle::create(&mStorage[4 + inWidthOfX]),
- inWidthOfX, inWidthOfX);
+ TransparentHandle::create(&mStorage[4 + widthOfX],
+ widthOfX * widthOfX * sizeof(double),
+ memoryController()),
+ widthOfX, widthOfX);
}
Array<double> mStorage;
View
200 src/modules/regress/logistic.cpp
@@ -76,24 +76,10 @@ AnyType stateToResult(AbstractDBInterface &db,
class LogisticRegressionCG::State {
public:
State(AnyType inArg)
- : mStorage(inArg.cloneIfImmutable()),
- iteration(&mStorage[0]),
- widthOfX(&mStorage[1]),
- coef(TransparentHandle::create(&mStorage[2]),
- widthOfX),
- dir(TransparentHandle::create(&mStorage[2 + widthOfX]),
- widthOfX),
- grad(TransparentHandle::create(&mStorage[2 + 2 * widthOfX]),
- widthOfX),
- beta(&mStorage[2 + 3 * widthOfX]),
-
- numRows(&mStorage[3 + 3 * widthOfX]),
- gradNew(TransparentHandle::create(&mStorage[4 + 3 * widthOfX]),
- widthOfX),
- X_transp_AX(TransparentHandle::create(&mStorage[4 + 4 * widthOfX]),
- widthOfX, widthOfX),
- logLikelihood(&mStorage[4 + widthOfX * widthOfX + 4 * widthOfX])
- { }
+ : mStorage(inArg.cloneIfImmutable()) {
+
+ rebind();
+ }
/**
* We define this function so that we can use State in the
@@ -112,23 +98,7 @@ class LogisticRegressionCG::State {
const uint16_t inWidthOfX) {
mStorage.rebind(inAllocator, boost::extents[ arraySize(inWidthOfX) ]);
- iteration.rebind(&mStorage[0]) = 0;
- widthOfX.rebind(&mStorage[1]) = inWidthOfX;
- coef.rebind(TransparentHandle::create(&mStorage[2]),
- widthOfX).zeros();
- dir.rebind(TransparentHandle::create(&mStorage[2 + widthOfX]),
- widthOfX).zeros();
- grad.rebind(TransparentHandle::create(&mStorage[2 + 2 * widthOfX]),
- widthOfX).zeros();
- beta.rebind(&mStorage[2 + 3 * widthOfX]) = 0;
-
- numRows.rebind(&mStorage[3 + 3 * widthOfX]);
- gradNew.rebind(TransparentHandle::create(&mStorage[4 + 3 * widthOfX]),
- widthOfX);
- X_transp_AX.rebind(TransparentHandle::create(&mStorage[4 + 4 * widthOfX]),
- widthOfX, widthOfX);
- logLikelihood.rebind(&mStorage[4 + widthOfX * widthOfX + 4 * widthOfX]);
- reset();
+ rebind(inWidthOfX);
}
/**
@@ -165,9 +135,59 @@ class LogisticRegressionCG::State {
}
private:
- static inline uint32_t arraySize(const uint16_t inWidthOfX) {
+ static inline uint64_t arraySize(const uint16_t inWidthOfX) {
return 5 + inWidthOfX * inWidthOfX + 4 * inWidthOfX;
}
+
+ /**
+ * @brief Who should own TransparentHandles pointing to slices of mStorage?
+ */
+ AbstractHandle::MemoryController memoryController() const {
+ AbstractHandle::MemoryController ctrl =
+ mStorage.memoryHandle()->memoryController();
+
+ return (ctrl == AbstractHandle::kSelf ? AbstractHandle::kLocal : ctrl);
+ }
+
+ /**
+ * @brief Rebind vector to a particular position in storage array
+ */
+ void inline rebindToPos(DoubleCol &inVec, size_t inPos) {
+ inVec.rebind(
+ TransparentHandle::create(&mStorage[inPos],
+ widthOfX * sizeof(double),
+ memoryController()),
+ widthOfX);
+ }
+
+ /**
+ * @brief Rebind to a new storage array
+ *
+ * @param inWidthOfX If this value is positive, use it as the number of
+ * independent variables. This is needed during initialization, when
+ * the storage array is still all zero, but we do already know the
+ * with of the design matrix.
+ */
+ void rebind(uint16_t inWidthOfX = 0) {
+ iteration.rebind(&mStorage[0]);
+ widthOfX.rebind(&mStorage[1]);
+
+ if (inWidthOfX != 0)
+ widthOfX = inWidthOfX;
+
+ rebindToPos(coef, 2);
+ rebindToPos(dir, 2 + widthOfX);
+ rebindToPos(grad, 2 + 2 * widthOfX);
+ beta.rebind(&mStorage[2 + 3 * widthOfX]);
+ numRows.rebind(&mStorage[3 + 3 * widthOfX]);
+ rebindToPos(gradNew, 4 + 3 * widthOfX);
+ X_transp_AX.rebind(
+ TransparentHandle::create(&mStorage[4 + 4 * widthOfX],
+ widthOfX * widthOfX * sizeof(double),
+ memoryController()),
+ widthOfX, widthOfX);
+ logLikelihood.rebind(&mStorage[4 + widthOfX * widthOfX + 4 * widthOfX]);
+ }
Array<double> mStorage;
@@ -183,13 +203,12 @@ class LogisticRegressionCG::State {
DoubleCol gradNew;
DoubleMat X_transp_AX;
Reference<double> logLikelihood;
-// Reference<double> dTHd;
};
/**
* @brief Logistic function
*/
-static double sigma(double x) {
+static inline double sigma(double x) {
return 1. / (1. + std::exp(-x));
}
@@ -204,7 +223,12 @@ AnyType LogisticRegressionCG::transition(AbstractDBInterface &db, AnyType args)
double y = *arg++ ? 1. : -1.;
DoubleRow_const x = *arg++;
if (state.numRows == 0) {
- state.initialize(db.allocator(AbstractAllocator::kAggregate), x.n_elem);
+ state.initialize(
+ db.allocator(
+ AbstractAllocator::kAggregate,
+ AbstractAllocator::kZero
+ ),
+ x.n_elem);
if (!arg->isNull()) {
const State previousState = *arg;
@@ -363,18 +387,10 @@ AnyType LogisticRegressionCG::result(AbstractDBInterface &db, AnyType args) {
class LogisticRegressionIRLS::State {
public:
State(AnyType inArg)
- : mStorage(inArg.cloneIfImmutable()),
- widthOfX(&mStorage[0]),
- coef(TransparentHandle::create(&mStorage[1]),
- widthOfX),
+ : mStorage(inArg.cloneIfImmutable()) {
- numRows(&mStorage[1 + widthOfX]),
- X_transp_Az(TransparentHandle::create(&mStorage[2 + widthOfX]),
- widthOfX),
- X_transp_AX(TransparentHandle::create(&mStorage[2 + 2 * widthOfX]),
- widthOfX, widthOfX),
- logLikelihood(&mStorage[2 + widthOfX * widthOfX + 2 * widthOfX])
- { }
+ rebind();
+ }
/**
* We define this function so that we can use State in the
@@ -385,7 +401,7 @@ class LogisticRegressionIRLS::State {
}
/**
- * @brief Initialize the conjugate-gradient state.
+ * @brief Initialize the iteratively-reweighted-least-squares state.
*
* This function is only called for the first iteration, for the first row.
*/
@@ -393,17 +409,7 @@ class LogisticRegressionIRLS::State {
const uint16_t inWidthOfX) {
mStorage.rebind(inAllocator, boost::extents[ arraySize(inWidthOfX) ]);
- widthOfX.rebind(&mStorage[0]) = inWidthOfX;
- coef.rebind(TransparentHandle::create(&mStorage[1]),
- widthOfX).zeros();
-
- numRows.rebind(&mStorage[1 + widthOfX]);
- X_transp_Az.rebind(TransparentHandle::create(&mStorage[2 + widthOfX]),
- widthOfX);
- X_transp_AX.rebind(TransparentHandle::create(&mStorage[2 + 2 * widthOfX]),
- widthOfX, widthOfX);
- logLikelihood.rebind(&mStorage[2 + widthOfX * widthOfX + 2 * widthOfX]);
- reset();
+ rebind(inWidthOfX);
}
/**
@@ -415,12 +421,14 @@ class LogisticRegressionIRLS::State {
}
/**
- * @brief Merge with another State object by copying the intra-iteration fields
+ * @brief Merge with another State object by copying the intra-iteration
+ * fields
*/
State &operator+=(const State &inOtherState) {
if (mStorage.size() != inOtherState.mStorage.size() ||
widthOfX != inOtherState.widthOfX)
- throw std::logic_error("Internal error: Incompatible transition states");
+ throw std::logic_error("Internal error: Incompatible transition "
+ "states");
numRows += inOtherState.numRows;
X_transp_Az += inOtherState.X_transp_Az;
@@ -430,6 +438,52 @@ class LogisticRegressionIRLS::State {
}
/**
+ * @brief Who should own TransparentHandles pointing to slices of mStorage?
+ */
+ AbstractHandle::MemoryController memoryController() const {
+ AbstractHandle::MemoryController ctrl =
+ mStorage.memoryHandle()->memoryController();
+
+ return (ctrl == AbstractHandle::kSelf ? AbstractHandle::kLocal : ctrl);
+ }
+
+ /**
+ * @brief Rebind vector to a particular position in storage array
+ */
+ void inline rebindToPos(DoubleCol &inVec, size_t inPos) {
+ inVec.rebind(
+ TransparentHandle::create(&mStorage[inPos],
+ widthOfX * sizeof(double),
+ memoryController()),
+ widthOfX);
+ }
+
+ /**
+ * @brief Rebind to a new storage array
+ *
+ * @param inWidthOfX If this value is positive, use it as the number of
+ * independent variables. This is needed during initialization, when
+ * the storage array is still all zero, but we do already know the
+ * with of the design matrix.
+ */
+ void rebind(uint16_t inWidthOfX = 0) {
+ widthOfX.rebind(&mStorage[0]);
+ if (inWidthOfX != 0)
+ widthOfX = inWidthOfX;
+
+ rebindToPos(coef, 1);
+
+ numRows.rebind(&mStorage[1 + widthOfX]);
+ rebindToPos(X_transp_Az, 2 + widthOfX);
+ X_transp_AX.rebind(
+ TransparentHandle::create(&mStorage[2 + 2 * widthOfX],
+ widthOfX * widthOfX * sizeof(double),
+ memoryController()),
+ widthOfX, widthOfX);
+ logLikelihood.rebind(&mStorage[2 + widthOfX * widthOfX + 2 * widthOfX]);
+ }
+
+ /**
* @brief Reset the inter-iteration fields.
*/
inline void reset() {
@@ -474,7 +528,11 @@ AnyType LogisticRegressionIRLS::transition(AbstractDBInterface &db,
throw std::invalid_argument("Design matrix is not finite.");
if (state.numRows == 0) {
- state.initialize(db.allocator(AbstractAllocator::kAggregate), x.n_elem);
+ state.initialize(
+ db.allocator(
+ AbstractAllocator::kAggregate,
+ AbstractAllocator::kZero),
+ x.n_elem);
if (!arg->isNull()) {
const State previousState = *arg;
@@ -515,7 +573,9 @@ AnyType LogisticRegressionIRLS::transition(AbstractDBInterface &db,
/**
* @brief Perform the perliminary aggregation function: Merge transition states
*/
-AnyType LogisticRegressionIRLS::mergeStates(AbstractDBInterface & /* db */, AnyType args) {
+AnyType LogisticRegressionIRLS::mergeStates(AbstractDBInterface & /* db */,
+ AnyType args) {
+
State stateLeft = args[0].cloneIfImmutable();
const State stateRight = args[1];
@@ -534,7 +594,9 @@ AnyType LogisticRegressionIRLS::mergeStates(AbstractDBInterface & /* db */, AnyT
/**
* @brief Perform the logistic-regression final step
*/
-AnyType LogisticRegressionIRLS::final(AbstractDBInterface & /* db */, AnyType args) {
+AnyType LogisticRegressionIRLS::final(AbstractDBInterface & /* db */,
+ AnyType args) {
+
// Argument from SQL call
State state = args[0].cloneIfImmutable();
@@ -552,7 +614,9 @@ AnyType LogisticRegressionIRLS::final(AbstractDBInterface & /* db */, AnyType ar
/**
* @brief Return the difference in log-likelihood between two states
*/
-AnyType LogisticRegressionIRLS::distance(AbstractDBInterface & /* db */, AnyType args) {
+AnyType LogisticRegressionIRLS::distance(AbstractDBInterface & /* db */,
+ AnyType args) {
+
const State stateLeft = args[0];
const State stateRight = args[1];
View
3  src/ports/postgres/dbconnector/PGAbstractType.cpp
@@ -59,7 +59,8 @@ AbstractTypeSPtr PGAbstractType::DatumToValue(bool inMemoryIsWritable,
switch (ARR_ELEMTYPE(pgArray)) {
case FLOAT8OID: {
- MemHandleSPtr memoryHandle(new PGArrayHandle(pgArray));
+ MemHandleSPtr memoryHandle(
+ new PGArrayHandle(pgArray, AbstractHandle::kGlobal));
if (inMemoryIsWritable) {
return AbstractTypeSPtr(
View
12 src/ports/postgres/dbconnector/PGAllocator.cpp
@@ -284,7 +284,17 @@ void PGAllocator::free(void *inPtr) const throw() {
RESUME_INTERRUPTS();
}
-
} // namespace dbconnector
+namespace dbal {
+
+/**
+ * @brief Return the default allocator used by operator new and operator delete.
+ */
+AbstractAllocator &defaultAllocator() {
+ return dbconnector::PGAllocator::defaultAllocator();
+}
+
+} // namespace dbal
+
} // namespace madlib
View
2  src/ports/postgres/dbconnector/PGAllocator.hpp
@@ -22,7 +22,7 @@ friend class PGInterface;
/**
* @brief Return the default allocator used by operator new and operator delete.
*/
- static PGAllocator &defaultAllocator() {
+ static inline PGAllocator &defaultAllocator() {
static PGAllocator sDefaultAllocator;
return sDefaultAllocator;
View
10 src/ports/postgres/dbconnector/PGArrayHandle.cpp
@@ -14,13 +14,13 @@ namespace dbconnector {
* @brief Constructor
*
* @param inArray PostgreSQL ArrayType struct
- * @param inCopyMem Copy array? If yes, the handle owns the memory and will
+ * @param inCtrl Copy array? If yes, the handle owns the memory and will
* take care of cleaning up.
*/
-PGArrayHandle::PGArrayHandle(ArrayType *inArray, bool inCopyMem)
- : mArray(inArray), mOwnsArray(inCopyMem) {
+PGArrayHandle::PGArrayHandle(ArrayType *inArray, MemoryController inCtrl)
+ : AbstractHandle(inCtrl), mArray(inArray) {
- if (inCopyMem) {
+ if (inCtrl == kSelf) {
// Use default allocator (this will allocate memory in the default
// postgres context, i.e., the function-call context)
@@ -31,7 +31,7 @@ PGArrayHandle::PGArrayHandle(ArrayType *inArray, bool inCopyMem)
}
PGArrayHandle::~PGArrayHandle() {
- if (mOwnsArray)
+ if (memoryController() == kSelf)
PGAllocator::defaultAllocator().free(mArray);
}
View
13 src/ports/postgres/dbconnector/PGArrayHandle.hpp
@@ -17,8 +17,10 @@ namespace dbconnector {
class PGArrayHandle : public AbstractHandle {
friend class PGAllocator;
public:
- PGArrayHandle(ArrayType *inArray, bool inCopyMem = false);
-
+ using AbstractHandle::MemoryController;
+
+ PGArrayHandle(ArrayType *inArray, MemoryController inCtrl = kLocal);
+
~PGArrayHandle();
virtual void *ptr() {
@@ -29,17 +31,12 @@ friend class PGAllocator;
return mArray;
}
- virtual void release() {
- mOwnsArray = false;
- }
-
virtual MemHandleSPtr clone() const {
- return MemHandleSPtr( new PGArrayHandle(mArray, true) );
+ return MemHandleSPtr( new PGArrayHandle(mArray, kSelf) );
}
protected:
ArrayType *mArray;
- bool mOwnsArray;
};
} // namespace dbconnector
View
2  src/ports/postgres/dbconnector/PGCommon.hpp
@@ -11,7 +11,7 @@
#ifndef MADLIB_POSTGRES_COMMON_HPP
#define MADLIB_POSTGRES_COMMON_HPP
-// Handle faist Boost assertions in a sophisticated way. The handler is
+// Handle failed Boost assertions in a sophisticated way. The handler is
// implemented in the MADlib core library.
#define BOOST_ENABLE_ASSERT_HANDLER
View
14 src/ports/postgres/dbconnector/PGToDatumConverter.cpp
@@ -258,17 +258,19 @@ void PGToDatumConverter::convertArray(const MemHandleSPtr &inHandle,
shared_ptr<PGArrayHandle> arrayHandle
= dynamic_pointer_cast<PGArrayHandle>(inHandle);
+
+ if (arrayHandle)
+ // We will not deallocate the storage used by the Array
+ // because we are returning a pointer to this storage, which is a
+ // PostgreSQL array!
+ // We are guaranteed that backend code will take care of
+ // deallocation. See MADLIB-250.
+ arrayHandle->release();
PG_TRY(); {
if (arrayHandle) {
- // We will not deallocate the storage used by the Array
- // because we are returning a pointer to this storage!
- // We are guaranteed that backend code will take care of
- // deallocation. See MADLIB-250.
- arrayHandle->release();
mConvertedValue = PointerGetDatum(arrayHandle->array());
} else {
- // FIXME: Check whether this code is used at all.
// If the Array does not use a PostgreSQL array
// as its storage, we have to create a new PostgreSQL array
// and copy the values (contruct_array() will do a copy).
Please sign in to comment.
Something went wrong with that request. Please try again.