From af88fdefcf489cb9c6674f45f31898c8cee8027c Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Wed, 25 Apr 2018 22:53:44 -0700 Subject: [PATCH] Incorporate C++ Buffer management and Seal global threadpool fix from arrow (#1950) --- python/build-wheel-macos.sh | 2 ++ src/common/cmake/Common.cmake | 12 +++++++++--- src/ray/object_manager/object_buffer_pool.cc | 2 +- .../test/object_manager_stress_test.cc | 6 +++--- thirdparty/scripts/build_arrow.sh | 8 ++------ thirdparty/scripts/build_flatbuffers.sh | 3 ++- 6 files changed, 19 insertions(+), 14 deletions(-) diff --git a/python/build-wheel-macos.sh b/python/build-wheel-macos.sh index 7559fd02c..8a56c59f8 100755 --- a/python/build-wheel-macos.sh +++ b/python/build-wheel-macos.sh @@ -53,6 +53,8 @@ for ((i=0; i<${#PY_VERSIONS[@]}; ++i)); do popd pushd python + # Setuptools on CentOS is too old to install arrow 0.9.0, therefore we upgrade. + $PIP_CMD install --upgrade setuptools # Install setuptools_scm because otherwise when building the wheel for # Python 3.6, we see an error. $PIP_CMD install -q setuptools_scm diff --git a/src/common/cmake/Common.cmake b/src/common/cmake/Common.cmake index 9e12ab9cc..b46bdeeb1 100644 --- a/src/common/cmake/Common.cmake +++ b/src/common/cmake/Common.cmake @@ -10,7 +10,12 @@ if(UNIX AND NOT APPLE) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -rdynamic") endif() -set(FLATBUFFERS_VERSION "1.7.1") +# The following is needed because in CentOS, the lib directory is named lib64 +if(EXISTS "/etc/redhat-release" AND CMAKE_SIZEOF_VOID_P EQUAL 8) + set(LIB_SUFFIX 64) +endif() + +set(FLATBUFFERS_VERSION "1.9.0") set(FLATBUFFERS_PREFIX "${CMAKE_BINARY_DIR}/flatbuffers_ep-prefix/src/flatbuffers_ep-install") if (NOT TARGET flatbuffers_ep) @@ -19,13 +24,14 @@ if (NOT TARGET flatbuffers_ep) CMAKE_ARGS "-DCMAKE_CXX_FLAGS=-fPIC" "-DCMAKE_INSTALL_PREFIX:PATH=${FLATBUFFERS_PREFIX}" - "-DFLATBUFFERS_BUILD_TESTS=OFF") + "-DFLATBUFFERS_BUILD_TESTS=OFF" + "-DCMAKE_BUILD_TYPE=RELEASE") endif() set(FBS_DEPENDS flatbuffers_ep) set(FLATBUFFERS_INCLUDE_DIR "${FLATBUFFERS_PREFIX}/include") -set(FLATBUFFERS_STATIC_LIB "${FLATBUFFERS_PREFIX}/lib/libflatbuffers.a") +set(FLATBUFFERS_STATIC_LIB "${FLATBUFFERS_PREFIX}/lib${LIB_SUFFIX}/libflatbuffers.a") set(FLATBUFFERS_COMPILER "${FLATBUFFERS_PREFIX}/bin/flatc") message(STATUS "Flatbuffers include dir: ${FLATBUFFERS_INCLUDE_DIR}") diff --git a/src/ray/object_manager/object_buffer_pool.cc b/src/ray/object_manager/object_buffer_pool.cc index beab84736..1ab9069bb 100644 --- a/src/ray/object_manager/object_buffer_pool.cc +++ b/src/ray/object_manager/object_buffer_pool.cc @@ -51,7 +51,7 @@ std::pair ObjectBufferPool::Ge RAY_CHECK(object_buffer.metadata->data() == object_buffer.data->data() + object_buffer.data->size()); RAY_CHECK(data_size == static_cast(object_buffer.data->size() + - object_buffer.metadata_size)); + object_buffer.metadata->size())); auto *data = const_cast(object_buffer.data->data()); uint64_t num_chunks = GetNumChunks(data_size); get_buffer_state_.emplace( diff --git a/src/ray/object_manager/test/object_manager_stress_test.cc b/src/ray/object_manager/test/object_manager_stress_test.cc index 8019725e4..350b37c4c 100644 --- a/src/ray/object_manager/test/object_manager_stress_test.cc +++ b/src/ray/object_manager/test/object_manager_stress_test.cc @@ -296,9 +296,9 @@ class StressTestObjectManager : public TestObjectManagerBase { plasma::ObjectBuffer object_buffer_2 = GetObject(client2, object_id_2); uint8_t *data_1 = const_cast(object_buffer_1.data->data()); uint8_t *data_2 = const_cast(object_buffer_2.data->data()); - ASSERT_EQ(object_buffer_1.data->size(), object_buffer_2.data_size); - ASSERT_EQ(object_buffer_1.metadata_size, object_buffer_2.metadata_size); - int64_t total_size = object_buffer_1.data->size() + object_buffer_1.metadata_size; + ASSERT_EQ(object_buffer_1.data->size(), object_buffer_2.data->size()); + ASSERT_EQ(object_buffer_1.metadata->size(), object_buffer_2.metadata->size()); + int64_t total_size = object_buffer_1.data->size() + object_buffer_1.metadata->size(); RAY_LOG(DEBUG) << "total_size " << total_size; for (int i = -1; ++i < total_size;) { ASSERT_TRUE(data_1[i] == data_2[i]); diff --git a/thirdparty/scripts/build_arrow.sh b/thirdparty/scripts/build_arrow.sh index 94020eb72..f2fbb5c1d 100755 --- a/thirdparty/scripts/build_arrow.sh +++ b/thirdparty/scripts/build_arrow.sh @@ -44,14 +44,10 @@ if [[ ! -d $TP_DIR/../python/ray/pyarrow_files/pyarrow ]]; then pushd $TP_DIR/build/arrow git fetch origin master - # The PR for this commit is https://github.com/apache/arrow/pull/1874. We + # The PR for this commit is https://github.com/apache/arrow/pull/1939. We # include the link here to make it easier to find the right commit because # Arrow often rewrites git history and invalidates certain commits. - git checkout 0f87c12d45250ee763ac8c43b7e57e8f06a0b9f3 - - # Revert https://github.com/apache/arrow/pull/1807, which unfortunately - # introduces the issue in https://issues.apache.org/jira/browse/ARROW-2448. - git revert --no-commit cf396867df6f1f93948c69ce10ceb0f95e399242 + git checkout 5f9cf9c96709f92e9ac4828cf3e106a165576ce7 cd cpp if [ ! -d "build" ]; then diff --git a/thirdparty/scripts/build_flatbuffers.sh b/thirdparty/scripts/build_flatbuffers.sh index 59f14a05e..539e06f4a 100755 --- a/thirdparty/scripts/build_flatbuffers.sh +++ b/thirdparty/scripts/build_flatbuffers.sh @@ -7,7 +7,7 @@ set -e TP_DIR=$(cd "$(dirname "${BASH_SOURCE:-$0}")"; pwd)/../ -FLATBUFFERS_VERSION=1.7.1 +FLATBUFFERS_VERSION=1.9.0 # Download and compile flatbuffers if it isn't already present. if [ ! -d $TP_DIR/pkg/flatbuffers ]; then @@ -20,6 +20,7 @@ if [ ! -d $TP_DIR/pkg/flatbuffers ]; then # Compile flatbuffers. pushd flatbuffers-$FLATBUFFERS_VERSION cmake -DCMAKE_CXX_FLAGS=-fPIC \ + -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_INSTALL_PREFIX:PATH=$TP_DIR/pkg/flatbuffers \ -DFLATBUFFERS_BUILD_TESTS=OFF make -j5