From 96760f837f116125c03398cf330c7bf68eb250de Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Tue, 4 Oct 2022 12:49:18 +0000 Subject: [PATCH 1/6] remove using brew llvm --- java/gandiva/src/main/cpp/jni_common.cc | 40 ++++++++++++++----------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/java/gandiva/src/main/cpp/jni_common.cc b/java/gandiva/src/main/cpp/jni_common.cc index ba0af1106b1..bf9ed7de72a 100644 --- a/java/gandiva/src/main/cpp/jni_common.cc +++ b/java/gandiva/src/main/cpp/jni_common.cc @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +#include #include #include @@ -712,9 +713,7 @@ class JavaResizableBuffer : public arrow::ResizableBuffer { Status Resize(const int64_t new_size, bool shrink_to_fit) override; - Status Reserve(const int64_t new_capacity) override { - return Status::NotImplemented("reserve not implemented"); - } + Status Reserve(const int64_t new_capacity) override; private: JNIEnv* env_; @@ -722,20 +721,10 @@ class JavaResizableBuffer : public arrow::ResizableBuffer { int32_t vector_idx_; }; -Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { - if (shrink_to_fit == true) { - return Status::NotImplemented("shrink not implemented"); - } - - if (ARROW_PREDICT_TRUE(new_size < capacity())) { - // no need to expand. - size_ = new_size; - return Status::OK(); - } - +Status JavaResizableBuffer::Reserve(const int64_t new_capacity) { // callback into java to expand the buffer - jobject ret = - env_->CallObjectMethod(jexpander_, vector_expander_method_, vector_idx_, new_size); + jobject ret = env_->CallObjectMethod(jexpander_, vector_expander_method_, vector_idx_, + new_capacity); if (env_->ExceptionCheck()) { env_->ExceptionDescribe(); env_->ExceptionClear(); @@ -744,14 +733,29 @@ Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { jlong ret_address = env_->GetLongField(ret, vector_expander_ret_address_); jlong ret_capacity = env_->GetLongField(ret, vector_expander_ret_capacity_); - DCHECK_GE(ret_capacity, new_size); data_ = reinterpret_cast(ret_address); - size_ = new_size; capacity_ = ret_capacity; return Status::OK(); } +Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { + if (shrink_to_fit == true) { + return Status::NotImplemented("shrink not implemented"); + } + + if (ARROW_PREDICT_TRUE(new_size < capacity())) { + // no need to expand. + size_ = new_size; + return Status::OK(); + } + + RETURN_NOT_OK(Reserve(new_size)); + DCHECK_GE(capacity_, new_size); + size_ = new_size; + return Status::OK(); +} + #define CHECK_OUT_BUFFER_IDX_AND_BREAK(idx, len) \ if (idx >= len) { \ status = gandiva::Status::Invalid("insufficient number of out_buf_addrs"); \ From cbd1bc477cf3b422aaf9bc97962c2c2851195b57 Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Tue, 4 Oct 2022 13:17:14 +0000 Subject: [PATCH 2/6] pin macos llvm to 14 --- .github/workflows/cpp.yml | 2 ++ .github/workflows/python.yml | 2 ++ .github/workflows/ruby.yml | 2 ++ dev/tasks/java-jars/github.yml | 2 ++ dev/tasks/verify-rc/github.macos.amd64.yml | 2 ++ 5 files changed, 10 insertions(+) diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index b420ec06395..511b307c43c 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -182,7 +182,9 @@ jobs: key: cpp-ccache-macos-${{ hashFiles('cpp/**') }} restore-keys: cpp-ccache-macos- - name: Build + # pin LLVM version on MacOS to 14 ARROW-17902 run: | + export LLVM_ROOT=$(brew --prefix llvm@14) ci/scripts/cpp_build.sh $(pwd) $(pwd)/build - name: Test shell: bash diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 884c2fc7367..05e08781ee0 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -167,7 +167,9 @@ jobs: -r python/requirements-test.txt - name: Build shell: bash + # pin LLVM version on MacOS to 14 ARROW-17902 run: | + export LLVM_ROOT=$(brew --prefix llvm@14) export PYTHON=python3 ci/scripts/cpp_build.sh $(pwd) $(pwd)/build ci/scripts/python_build.sh $(pwd) $(pwd)/build diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 22045700346..63141c1b28d 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -169,7 +169,9 @@ jobs: key: ruby-ccache-macos-${{ hashFiles('cpp/**') }} restore-keys: ruby-ccache-macos- - name: Build C++ + # pin LLVM version on MacOS to 14 ARROW-17902 run: | + export LLVM_ROOT=$(brew --prefix llvm@14) ci/scripts/cpp_build.sh $(pwd) $(pwd)/build - name: Build GLib run: | diff --git a/dev/tasks/java-jars/github.yml b/dev/tasks/java-jars/github.yml index 6f7fdc82d5f..6d2ef9d9465 100644 --- a/dev/tasks/java-jars/github.yml +++ b/dev/tasks/java-jars/github.yml @@ -69,7 +69,9 @@ jobs: run: | arrow/ci/scripts/ccache_setup.sh - name: Build C++ libraries + # pin LLVM version on MacOS to 14 ARROW-17902 run: | + export LLVM_ROOT=$(brew --prefix llvm@14) set -e arrow/ci/scripts/java_jni_macos_build.sh \ $GITHUB_WORKSPACE/arrow \ diff --git a/dev/tasks/verify-rc/github.macos.amd64.yml b/dev/tasks/verify-rc/github.macos.amd64.yml index cdd61702899..550a0afa440 100644 --- a/dev/tasks/verify-rc/github.macos.amd64.yml +++ b/dev/tasks/verify-rc/github.macos.amd64.yml @@ -70,5 +70,7 @@ jobs: {% if use_conda %} USE_CONDA: 1 {% endif %} + # pin LLVM version on MacOS to 14 ARROW-17902 run: | + export LLVM_ROOT=$(brew --prefix llvm@14) arrow/dev/release/verify-release-candidate.sh {{ release|default("") }} {{ rc|default("") }} From a29148be011860a770ce28591de96447e69ecff5 Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Tue, 4 Oct 2022 13:38:02 +0000 Subject: [PATCH 3/6] revert java gandiva change --- java/gandiva/src/main/cpp/jni_common.cc | 40 +++++++++++-------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/java/gandiva/src/main/cpp/jni_common.cc b/java/gandiva/src/main/cpp/jni_common.cc index bf9ed7de72a..ba0af1106b1 100644 --- a/java/gandiva/src/main/cpp/jni_common.cc +++ b/java/gandiva/src/main/cpp/jni_common.cc @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -#include #include #include @@ -713,7 +712,9 @@ class JavaResizableBuffer : public arrow::ResizableBuffer { Status Resize(const int64_t new_size, bool shrink_to_fit) override; - Status Reserve(const int64_t new_capacity) override; + Status Reserve(const int64_t new_capacity) override { + return Status::NotImplemented("reserve not implemented"); + } private: JNIEnv* env_; @@ -721,24 +722,6 @@ class JavaResizableBuffer : public arrow::ResizableBuffer { int32_t vector_idx_; }; -Status JavaResizableBuffer::Reserve(const int64_t new_capacity) { - // callback into java to expand the buffer - jobject ret = env_->CallObjectMethod(jexpander_, vector_expander_method_, vector_idx_, - new_capacity); - if (env_->ExceptionCheck()) { - env_->ExceptionDescribe(); - env_->ExceptionClear(); - return Status::OutOfMemory("buffer expand failed in java"); - } - - jlong ret_address = env_->GetLongField(ret, vector_expander_ret_address_); - jlong ret_capacity = env_->GetLongField(ret, vector_expander_ret_capacity_); - - data_ = reinterpret_cast(ret_address); - capacity_ = ret_capacity; - return Status::OK(); -} - Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { if (shrink_to_fit == true) { return Status::NotImplemented("shrink not implemented"); @@ -750,9 +733,22 @@ Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { return Status::OK(); } - RETURN_NOT_OK(Reserve(new_size)); - DCHECK_GE(capacity_, new_size); + // callback into java to expand the buffer + jobject ret = + env_->CallObjectMethod(jexpander_, vector_expander_method_, vector_idx_, new_size); + if (env_->ExceptionCheck()) { + env_->ExceptionDescribe(); + env_->ExceptionClear(); + return Status::OutOfMemory("buffer expand failed in java"); + } + + jlong ret_address = env_->GetLongField(ret, vector_expander_ret_address_); + jlong ret_capacity = env_->GetLongField(ret, vector_expander_ret_capacity_); + DCHECK_GE(ret_capacity, new_size); + + data_ = reinterpret_cast(ret_address); size_ = new_size; + capacity_ = ret_capacity; return Status::OK(); } From bc3f06b2ed7e599190177c0d325819c5d29943af Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Tue, 4 Oct 2022 14:08:47 +0000 Subject: [PATCH 4/6] fix mac rc conda if --- dev/tasks/verify-rc/github.macos.amd64.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev/tasks/verify-rc/github.macos.amd64.yml b/dev/tasks/verify-rc/github.macos.amd64.yml index 550a0afa440..ae1d10d942f 100644 --- a/dev/tasks/verify-rc/github.macos.amd64.yml +++ b/dev/tasks/verify-rc/github.macos.amd64.yml @@ -72,5 +72,7 @@ jobs: {% endif %} # pin LLVM version on MacOS to 14 ARROW-17902 run: | + {% if not use_conda %} export LLVM_ROOT=$(brew --prefix llvm@14) + {% endif %} arrow/dev/release/verify-release-candidate.sh {{ release|default("") }} {{ rc|default("") }} From 1f56c75f7b3ca02fe8ccf0a65964d25de6330b61 Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Wed, 5 Oct 2022 09:44:07 +0000 Subject: [PATCH 5/6] Implement reserve for gandiva java buffer --- java/gandiva/src/main/cpp/jni_common.cc | 40 ++++++++++++++----------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/java/gandiva/src/main/cpp/jni_common.cc b/java/gandiva/src/main/cpp/jni_common.cc index ba0af1106b1..bf9ed7de72a 100644 --- a/java/gandiva/src/main/cpp/jni_common.cc +++ b/java/gandiva/src/main/cpp/jni_common.cc @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +#include #include #include @@ -712,9 +713,7 @@ class JavaResizableBuffer : public arrow::ResizableBuffer { Status Resize(const int64_t new_size, bool shrink_to_fit) override; - Status Reserve(const int64_t new_capacity) override { - return Status::NotImplemented("reserve not implemented"); - } + Status Reserve(const int64_t new_capacity) override; private: JNIEnv* env_; @@ -722,20 +721,10 @@ class JavaResizableBuffer : public arrow::ResizableBuffer { int32_t vector_idx_; }; -Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { - if (shrink_to_fit == true) { - return Status::NotImplemented("shrink not implemented"); - } - - if (ARROW_PREDICT_TRUE(new_size < capacity())) { - // no need to expand. - size_ = new_size; - return Status::OK(); - } - +Status JavaResizableBuffer::Reserve(const int64_t new_capacity) { // callback into java to expand the buffer - jobject ret = - env_->CallObjectMethod(jexpander_, vector_expander_method_, vector_idx_, new_size); + jobject ret = env_->CallObjectMethod(jexpander_, vector_expander_method_, vector_idx_, + new_capacity); if (env_->ExceptionCheck()) { env_->ExceptionDescribe(); env_->ExceptionClear(); @@ -744,14 +733,29 @@ Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { jlong ret_address = env_->GetLongField(ret, vector_expander_ret_address_); jlong ret_capacity = env_->GetLongField(ret, vector_expander_ret_capacity_); - DCHECK_GE(ret_capacity, new_size); data_ = reinterpret_cast(ret_address); - size_ = new_size; capacity_ = ret_capacity; return Status::OK(); } +Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { + if (shrink_to_fit == true) { + return Status::NotImplemented("shrink not implemented"); + } + + if (ARROW_PREDICT_TRUE(new_size < capacity())) { + // no need to expand. + size_ = new_size; + return Status::OK(); + } + + RETURN_NOT_OK(Reserve(new_size)); + DCHECK_GE(capacity_, new_size); + size_ = new_size; + return Status::OK(); +} + #define CHECK_OUT_BUFFER_IDX_AND_BREAK(idx, len) \ if (idx >= len) { \ status = gandiva::Status::Invalid("insufficient number of out_buf_addrs"); \ From 34c70613cdecd3ef6d463988d31ade227154c523 Mon Sep 17 00:00:00 2001 From: Jin Shang Date: Sun, 9 Oct 2022 00:20:32 +0000 Subject: [PATCH 6/6] rebase master and pr feedback --- .github/workflows/cpp.yml | 2 -- .github/workflows/python.yml | 2 -- .github/workflows/ruby.yml | 2 -- dev/tasks/java-jars/github.yml | 2 -- dev/tasks/verify-rc/github.macos.amd64.yml | 4 ---- java/gandiva/src/main/cpp/jni_common.cc | 4 ++-- 6 files changed, 2 insertions(+), 14 deletions(-) diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index 511b307c43c..b420ec06395 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -182,9 +182,7 @@ jobs: key: cpp-ccache-macos-${{ hashFiles('cpp/**') }} restore-keys: cpp-ccache-macos- - name: Build - # pin LLVM version on MacOS to 14 ARROW-17902 run: | - export LLVM_ROOT=$(brew --prefix llvm@14) ci/scripts/cpp_build.sh $(pwd) $(pwd)/build - name: Test shell: bash diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 05e08781ee0..884c2fc7367 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -167,9 +167,7 @@ jobs: -r python/requirements-test.txt - name: Build shell: bash - # pin LLVM version on MacOS to 14 ARROW-17902 run: | - export LLVM_ROOT=$(brew --prefix llvm@14) export PYTHON=python3 ci/scripts/cpp_build.sh $(pwd) $(pwd)/build ci/scripts/python_build.sh $(pwd) $(pwd)/build diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 63141c1b28d..22045700346 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -169,9 +169,7 @@ jobs: key: ruby-ccache-macos-${{ hashFiles('cpp/**') }} restore-keys: ruby-ccache-macos- - name: Build C++ - # pin LLVM version on MacOS to 14 ARROW-17902 run: | - export LLVM_ROOT=$(brew --prefix llvm@14) ci/scripts/cpp_build.sh $(pwd) $(pwd)/build - name: Build GLib run: | diff --git a/dev/tasks/java-jars/github.yml b/dev/tasks/java-jars/github.yml index 6d2ef9d9465..6f7fdc82d5f 100644 --- a/dev/tasks/java-jars/github.yml +++ b/dev/tasks/java-jars/github.yml @@ -69,9 +69,7 @@ jobs: run: | arrow/ci/scripts/ccache_setup.sh - name: Build C++ libraries - # pin LLVM version on MacOS to 14 ARROW-17902 run: | - export LLVM_ROOT=$(brew --prefix llvm@14) set -e arrow/ci/scripts/java_jni_macos_build.sh \ $GITHUB_WORKSPACE/arrow \ diff --git a/dev/tasks/verify-rc/github.macos.amd64.yml b/dev/tasks/verify-rc/github.macos.amd64.yml index ae1d10d942f..cdd61702899 100644 --- a/dev/tasks/verify-rc/github.macos.amd64.yml +++ b/dev/tasks/verify-rc/github.macos.amd64.yml @@ -70,9 +70,5 @@ jobs: {% if use_conda %} USE_CONDA: 1 {% endif %} - # pin LLVM version on MacOS to 14 ARROW-17902 run: | - {% if not use_conda %} - export LLVM_ROOT=$(brew --prefix llvm@14) - {% endif %} arrow/dev/release/verify-release-candidate.sh {{ release|default("") }} {{ rc|default("") }} diff --git a/java/gandiva/src/main/cpp/jni_common.cc b/java/gandiva/src/main/cpp/jni_common.cc index bf9ed7de72a..43db266ff56 100644 --- a/java/gandiva/src/main/cpp/jni_common.cc +++ b/java/gandiva/src/main/cpp/jni_common.cc @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. -#include #include #include @@ -28,6 +27,7 @@ #include #include +#include #include #include #include @@ -744,7 +744,7 @@ Status JavaResizableBuffer::Resize(const int64_t new_size, bool shrink_to_fit) { return Status::NotImplemented("shrink not implemented"); } - if (ARROW_PREDICT_TRUE(new_size < capacity())) { + if (ARROW_PREDICT_TRUE(new_size <= capacity())) { // no need to expand. size_ = new_size; return Status::OK();