Repository: madlib
Updated Branches:
refs/heads/master 3540a5603 -> d62e5516b
Allocator: Remove 16-byte alignment in GPDB 6
Findings:
1. MADlib performs a 16-byte alignment for pointers returned by palloc.
2. Postgres prepends a small (16 byte usually) header before every
pointer which includes
a. the memory context and
b. the size of the memory allocation.
3. Greenplum 6+ tweaks that scheme a little: instead of the memory context,
the header tracks a "shared header" which points to another struct with
richer information (aside from the memory context).
4. Postgres calls MemoryContextContains both with the final func
for an aggregate and final function for a windowed aggregate.
5. Currently Postgres always concludes that the datum from MADlib is
allocated outside of the context and makes an extra copy. In
Greenplum, MemoryContextContains needs to dereference the shared header.
This is a problem since the pointer has been shifted and the function is
getting a bad header.
In this commit, we disable the pointer alignment for GPDB 6+ to avoid
failure in this check. Further, we also have to disable vectorization in
Eigen since it does not work when pointers are not 16-byte aligned.
Closes #319
Co-authored-by: Jesse Zhang <[email protected]>
Co-authored-by: Nandish Jayaram <[email protected]>
Project: http://git-wip-us.apache.org/repos/asf/madlib/repo
Commit: http://git-wip-us.apache.org/repos/asf/madlib/commit/d62e5516
Tree: http://git-wip-us.apache.org/repos/asf/madlib/tree/d62e5516
Diff: http://git-wip-us.apache.org/repos/asf/madlib/diff/d62e5516
Branch: refs/heads/master
Commit: d62e5516bc6741beee18678da1b9b3e6cc95cdcf
Parents: 3540a56
Author: Rahul Iyer <[email protected]>
Authored: Wed Sep 12 16:59:59 2018 -0700
Committer: Rahul Iyer <[email protected]>
Committed: Tue Sep 18 11:46:45 2018 -0700
----------------------------------------------------------------------
src/ports/greenplum/dbconnector/dbconnector.hpp | 17 +++++++++++++++++
src/ports/postgres/dbconnector/Allocator_impl.hpp | 10 +++++-----
2 files changed, 22 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/madlib/blob/d62e5516/src/ports/greenplum/dbconnector/dbconnector.hpp
----------------------------------------------------------------------
diff --git a/src/ports/greenplum/dbconnector/dbconnector.hpp
b/src/ports/greenplum/dbconnector/dbconnector.hpp
index 9c38ef6..d06b154 100644
--- a/src/ports/greenplum/dbconnector/dbconnector.hpp
+++ b/src/ports/greenplum/dbconnector/dbconnector.hpp
@@ -32,6 +32,23 @@ extern "C" {
#include "Compatibility.hpp"
+#if GP_VERSION_NUM >= 60000
+ // MADlib aligns the pointers returned by palloc() to 16-byte boundaries
+ // (see Allocator_impl.hpp). This is done to allow Eigen vectorization
(see
+ // http://eigen.tuxfamily.org/index.php?title=FAQ#Vectorization for more
+ // info). This vectorization has to be explicitly disabled if pointers are
+ // not 16-byte aligned. Further, the pointer realignment invalidates a
+ // header that palloc creates just prior to the pointer address. Greenplum
+ // after commit f62bd1c fails due to this invalid header. Hence, the
+ // pointer realignment and Eigen vectorization is disabled below for
+ // Greenplum 6 and above.
+
+ // See http://eigen.tuxfamily.org/dox/group__TopicUnalignedArrayAssert.html
+ // for steps to disable vectorization
+ #define EIGEN_DONT_VECTORIZE
+ #define EIGEN_DISABLE_UNALIGNED_ARRAY_ASSERT
+#endif
+
#include "../../postgres/dbconnector/dbconnector.hpp"
#endif // defined(MADLIB_GREENPLUM_DBCONNECTOR_HPP)
http://git-wip-us.apache.org/repos/asf/madlib/blob/d62e5516/src/ports/postgres/dbconnector/Allocator_impl.hpp
----------------------------------------------------------------------
diff --git a/src/ports/postgres/dbconnector/Allocator_impl.hpp
b/src/ports/postgres/dbconnector/Allocator_impl.hpp
index 4c44207..996117b 100644
--- a/src/ports/postgres/dbconnector/Allocator_impl.hpp
+++ b/src/ports/postgres/dbconnector/Allocator_impl.hpp
@@ -211,7 +211,7 @@ template <dbal::ZeroMemory ZM>
inline
void *
Allocator::internalPalloc(size_t inSize) const {
-#if MAXIMUM_ALIGNOF >= 16
+#if MAXIMUM_ALIGNOF >= 16 || defined EIGEN_DONT_VECTORIZE
return (ZM == dbal::DoZero) ? palloc0(inSize) : palloc(inSize);
#else
if (inSize > std::numeric_limits<size_t>::max() - 16)
@@ -221,7 +221,7 @@ Allocator::internalPalloc(size_t inSize) const {
const size_t size = inSize + 16;
void *raw = (ZM == dbal::DoZero) ? palloc0(size) : palloc(size);
return makeAligned(raw);
-#endif
+#endif // MAXIMUM_ALIGNOF >= 16
}
/**
@@ -243,7 +243,7 @@ template <dbal::ZeroMemory ZM>
inline
void *
Allocator::internalRePalloc(void *inPtr, size_t inSize) const {
-#if MAXIMUM_ALIGNOF >= 16
+#if MAXIMUM_ALIGNOF >= 16 || defined EIGEN_DONT_VECTORIZE
return repalloc(inPtr, inSize);
#else
if (inSize > std::numeric_limits<size_t>::max() - 16) {
@@ -262,7 +262,7 @@ Allocator::internalRePalloc(void *inPtr, size_t inSize)
const {
}
return makeAligned(raw);
-#endif
+#endif // MAXIMUM_ALIGNOF >= 16
}
/**
@@ -298,7 +298,7 @@ Allocator::makeAligned(void *inPtr) const {
inline
void *
Allocator::unaligned(void *inPtr) const {
-#if MAXIMUM_ALIGNOF >= 16
+#if MAXIMUM_ALIGNOF >= 16 || defined EIGEN_DONT_VECTORIZE
return inPtr;
#else
return (*(reinterpret_cast<void**>(inPtr) - 1));
