GH-34113: [C++][Thirdparty] Bump zstd to v1.5.4 by mapleFU · Pull Request #34114 · apache/arrow

mapleFU · 2023-02-10T06:22:34Z

Rationale for this change

Zstd has release it's latest version, v1.5.4, which containing a lots of performance improvement under x86 and aarch64. The release note can be seen here: https://github.com/facebook/zstd/releases/tag/v1.5.4

What changes are included in this PR?

update zstd tag in thirdparty

Are these changes tested?

no more tests added

Are there any user-facing changes?

no

Closes: [C++] Upgrade zstd to v1.5.4 #34113

github-actions · 2023-02-10T06:22:58Z

Closes: [C++] Upgrade zstd to v1.5.4 #34113

github-actions · 2023-02-10T06:23:01Z

⚠️ GitHub issue #34113 has been automatically assigned in GitHub to PR creator.

mapleFU · 2023-02-10T06:41:08Z

Interesting, why C++ / AMD64 Windows 2019 C++17 (pull_request) failed... Let me find out the reason

mapleFU · 2023-02-10T06:43:17Z

Seems it downloads: https://github.com/facebook/zstd/archive/v1.5.4.tar.gz
the actual is: https://github.com/facebook/zstd/releases/download/v1.5.4/zstd-1.5.4.tar.gz

They have different hash tags

mapleFU · 2023-02-10T08:55:22Z

@kou Hi kou, sorry for disturbing you. I find a naive problem:

Currently we download from "github.com/facebook/zstd/archive/", and it hash tags differ from hashtag in release note. Currently we use "https://github.com/facebook/zstd/archive/${ARROW_ZSTD_BUILD_VERSION}.tar.gz", and can have ARROW_ZSTD_BUILD_VERSION like v1.5.2 or v1.5.4.

The release notes have link like https://github.com/facebook/zstd/releases/download/v{ARROW_ZSTD_BUILD_VERSION}/zstd-{ARROW_ZSTD_BUILD_VERSION}.tar.gz. Note that {ARROW_ZSTD_BUILD_VERSION} is like 1.5.2 or 1.5.4 without v.

Here I have 2 solving ways:

Just use "github.com/facebook/zstd/archive/", and recount a sha256sum
Use "https://github.com/facebook/zstd/releases/download/", and use https://github.com/facebook/zstd/releases/download/{ARROW_ZSTD_BUILD_VERSION}/zstd-{remove_prefix(ARROW_ZSTD_BUILD_VERSION)}.tar.gz

kou · 2023-02-10T21:32:21Z

Let's use the official one ( https://github.com/facebook/zstd/releases/download/v{ARROW_ZSTD_BUILD_VERSION}/zstd-{ARROW_ZSTD_BUILD_VERSION}.tar.gz )!

mapleFU · 2023-02-11T07:06:47Z

@kou All tests passed, would you mind check it?

kou

+1

Thanks!

assignUser · 2023-02-11T23:49:23Z

Let's use the official one ( https://github.com/facebook/zstd/releases/download/v{ARROW_ZSTD_BUILD_VERSION}/zstd-{ARROW_ZSTD_BUILD_VERSION}.tar.gz )!

that has the advantage of being hash stable unlike the archive links :D

ursabot · 2023-02-12T00:53:20Z

Benchmark runs are scheduled for baseline = b056e07 and contender = 3e1aaa5. 3e1aaa5 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.58% ⬆️0.98%] test-mac-arm
[Finished ⬇️0.51% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.32% ⬆️0.41%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 3e1aaa5c ec2-t3-xlarge-us-east-2
[Finished] 3e1aaa5c test-mac-arm
[Finished] 3e1aaa5c ursa-i9-9960x
[Finished] 3e1aaa5c ursa-thinkcentre-m75q
[Finished] b056e07b ec2-t3-xlarge-us-east-2
[Failed] b056e07b test-mac-arm
[Finished] b056e07b ursa-i9-9960x
[Finished] b056e07b ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

ursabot · 2023-02-12T00:54:58Z

['Python', 'R'] benchmarks have high level of regressions.
test-mac-arm

kou · 2023-02-12T06:38:13Z

Oh, this broke example-cpp-minimal-build-static...:

https://github.com/ursacomputing/crossbow/actions/runs/4153851721/jobs/7185765610#step:3:1131

[ 25%] Performing configure step for 'zstd_ep'
CMake Error at /build/arrow/zstd_ep-prefix/src/zstd_ep-stamp/zstd_ep-configure-RELEASE.cmake:37 (message):
-- stdout output is:
  Command failed: 1
-- The C compiler identification is GNU 9.4.0
-- The ASM compiler identification is GNU
-- Found assembler: /usr/bin/cc
-- The CXX compiler identification is GNU 9.4.0
-- Check for working C compiler: /usr/bin/cc

-- Check for working C compiler: /usr/bin/cc -- works
   '/usr/bin/cmake' '-DCMAKE_C_COMPILER=/usr/bin/cc' '-DCMAKE_CXX_COMPILER=/usr/bin/c++' '-DCMAKE_AR=/usr/bin/ar' '-DCMAKE_RANLIB=/usr/bin/ranlib' '-DBUILD_SHARED_LIBS=OFF' '-DBUILD_STATIC_LIBS=ON' '-DBUILD_TESTING=OFF' '-DCMAKE_BUILD_TYPE=RELEASE' '-DCMAKE_CXX_FLAGS= -fdiagnostics-color=always -fPIC' '-DCMAKE_CXX_FLAGS_DEBUG=-g -O0 -ggdb -Wno-error' '-DCMAKE_CXX_FLAGS_MISIZEREL=-Os -DNDEBUG' '-DCMAKE_CXX_FLAGS_RELEASE=-O3 -DNDEBUG -O2 -ftree-vectorize' '-DCMAKE_CXX_FLAGS_RELWITHDEBINFO=-O2 -g -DNDEBUG -O2 -ftree-vectorize  -O0 -ggdb' '-DCMAKE_CXX_STANDARD=17' '-DCMAKE_C_FLAGS= -fPIC' '-DCMAKE_C_FLAGS_DEBUG=-g -O0 -ggdb -Wno-error' '-DCMAKE_C_FLAGS_MISIZEREL=-Os -DNDEBUG' '-DCMAKE_C_FLAGS_RELEASE=-O3 -DNDEBUG -O2 -ftree-vectorize' '-DCMAKE_C_FLAGS_RELWITHDEBINFO=-O2 -g -DNDEBUG -O2 -ftree-vectorize  -O0 -ggdb' '-DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=' '-DCMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY=' '-DCMAKE_INSTALL_LIBDIR=lib' '-DCMAKE_VERBOSE_MAKEFILE=FALSE' '-DCMAKE_INSTALL_PREFIX=/build/arrow/zstd_ep-install' '-DZSTD_BUILD_PROGRAMS=OFF' '-DZSTD_BUILD_SHARED=OFF' '-DZSTD_BUILD_STATIC=ON' '-DZSTD_MULTITHREAD_SUPPORT=OFF' '-GUnix Makefiles' '/build/arrow/zstd_ep-prefix/src/zstd_ep/build/cmake'
-- Detecting C compiler ABI info

-- Detecting C compiler ABI info - done
  See also
-- Detecting C compile features

-- Detecting C compile features - done
    /build/arrow/zstd_ep-prefix/src/zstd_ep-stamp/zstd_ep-configure-*.log
-- Check for working CXX compiler: /usr/bin/c++

-- Check for working CXX compiler: /usr/bin/c++ -- works

-- Detecting CXX compiler ABI info
CMake Error at /build/arrow/zstd_ep-prefix/src/zstd_ep-stamp/zstd_ep-configure-RELEASE.cmake:47 (message):
-- Detecting CXX compiler ABI info - done
  Stopping after outputting logs.
-- Detecting CXX compile features

-- Detecting CXX compile features - done

-- ZSTD VERSION: 1.5.4
make[2]: *** [CMakeFiles/zstd_ep.dir/build.make:108: zstd_ep-prefix/src/zstd_ep-stamp/zstd_ep-configure] Error 1
-- Performing Test C_FLAG_WALL
make[1]: *** [CMakeFiles/Makefile2:1450: CMakeFiles/zstd_ep.dir/all] Error 2
-- Performing Test C_FLAG_WALL - Success
make[1]: *** Waiting for unfinished jobs....
-- Performing Test CXX_FLAG_WALL
-- Performing Test CXX_FLAG_WALL - Success
-- Performing Test C_FLAG_WEXTRA
-- Performing Test C_FLAG_WEXTRA - Success
-- Performing Test CXX_FLAG_WEXTRA
-- Performing Test CXX_FLAG_WEXTRA - Success
-- Performing Test C_FLAG_WUNDEF
-- Performing Test C_FLAG_WUNDEF - Success
-- Performing Test CXX_FLAG_WUNDEF
-- Performing Test CXX_FLAG_WUNDEF - Success
-- Performing Test C_FLAG_WSHADOW
-- Performing Test C_FLAG_WSHADOW - Success
-- Performing Test CXX_FLAG_WSHADOW
-- Performing Test CXX_FLAG_WSHADOW - Success
-- Performing Test C_FLAG_WCAST_ALIGN
-- Performing Test C_FLAG_WCAST_ALIGN - Success
-- Performing Test CXX_FLAG_WCAST_ALIGN
-- Performing Test CXX_FLAG_WCAST_ALIGN - Success
-- Performing Test C_FLAG_WCAST_QUAL
-- Performing Test C_FLAG_WCAST_QUAL - Success
-- Performing Test CXX_FLAG_WCAST_QUAL
-- Performing Test CXX_FLAG_WCAST_QUAL - Success
-- Performing Test C_FLAG_WSTRICT_PROTOTYPES
-- Performing Test C_FLAG_WSTRICT_PROTOTYPES - Success
-- Configuring incomplete, errors occurred!
See also "/build/arrow/zstd_ep-prefix/src/zstd_ep-build/CMakeFiles/CMakeOutput.log".

-- stderr output is:
CMake Error at CMakeModules/AddZstdCompilationFlags.cmake:3 (include):
  include could not find load file:

    CheckLinkerFlag
Call Stack (most recent call first):
  CMakeLists.txt:70 (include)


CMake Error at CMakeModules/AddZstdCompilationFlags.cmake:23 (CHECK_LINKER_FLAG):
  Unknown CMake command "CHECK_LINKER_FLAG".
Call Stack (most recent call first):
  CMakeModules/AddZstdCompilationFlags.cmake:57 (EnableCompilerFlag)
  CMakeLists.txt:71 (ADD_ZSTD_COMPILATION_FLAGS)

mapleFU · 2023-02-12T06:48:44Z

Well, didn't CI trigger building the example? I'll take a look tonight

mapleFU · 2023-02-12T07:27:41Z

I run

export ARROW_BUILD_DIR=$(pwd)/arrow-build
export EXAMPLE_BUILD_DIR=$(pwd)/example

./run_static.sh

On my macos, it build success. I'll try it later on my wsl ubuntu @kou

Or did you find out the reason will it failed?

mapleFU · 2023-02-12T07:32:25Z

Oh, find out why. https://cmake.org/cmake/help/latest/module/CheckLinkerFlag.html
This is introduced in CMake 3.18, but our minimal build is 3.0... The piece of code using CheckLinkerFlag is introduced in v1.5.4 in zstd.

mapleFU · 2023-02-12T07:33:38Z

Get:79 http://archive.ubuntu.com/ubuntu focal-updates/main amd64 cmake amd64 3.16.3-1ubuntu1.20.04.1 [3668 kB]

And the version we install is 3.16.3. So, it failed @kou .

Should we fix it with higher install version?

kou · 2023-02-12T07:37:49Z

Thanks for taking a look at this.
Could you open a new issue for this?

mapleFU · 2023-02-12T07:42:51Z

Create an issue here: #34148

### Rationale for this change This patch part reverts #34114 . But it didn't revert the logic for url and sha256 change. ### What changes are included in this PR? zstd version revert back to v1.5.2 ### Are these changes tested? no ### Are there any user-facing changes? no * Closes: #34148 Authored-by: mwish <maplewish117@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

mapleFU marked this pull request as ready for review February 10, 2023 06:22

github-actions bot added the Component: C++ label Feb 10, 2023

Bump zstd to v1.5.4

421d5d4

mapleFU force-pushed the arrow-cpp-upgrade-zstd branch from b994354 to 421d5d4 Compare February 10, 2023 08:06

mapleFU added 2 commits February 10, 2023 16:28

update zstd url rule

94d2582

update Thirdparty toolchain

b79cb28

Merge branch 'master' into arrow-cpp-upgrade-zstd

2ea0136

mapleFU force-pushed the arrow-cpp-upgrade-zstd branch from 205e138 to 2ea0136 Compare February 11, 2023 05:58

kou approved these changes Feb 11, 2023

View reviewed changes

kou merged commit 3e1aaa5 into apache:master Feb 11, 2023

mapleFU deleted the arrow-cpp-upgrade-zstd branch February 12, 2023 04:38

mapleFU mentioned this pull request Feb 12, 2023

[C++] Minimal build failed in crossbow because of zstd v1.5.4 requires CMake v3.18 #34148

Closed

kou mentioned this pull request Feb 15, 2023

GH-34148: [C++][Thirdparty] make zstd require CMake 3.18 #34149

Closed

mapleFU mentioned this pull request Feb 15, 2023

GH-34148: [C++] Revert zstd back to 1.5.2 #34190

Merged

Conversation

mapleFU commented Feb 10, 2023 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions bot commented Feb 10, 2023

Uh oh!

github-actions bot commented Feb 10, 2023

Uh oh!

mapleFU commented Feb 10, 2023

Uh oh!

mapleFU commented Feb 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mapleFU commented Feb 10, 2023

Uh oh!

kou commented Feb 10, 2023

Uh oh!

mapleFU commented Feb 11, 2023

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

assignUser commented Feb 11, 2023

Uh oh!

ursabot commented Feb 12, 2023

Uh oh!

ursabot commented Feb 12, 2023

Uh oh!

kou commented Feb 12, 2023

Uh oh!

mapleFU commented Feb 12, 2023

Uh oh!

mapleFU commented Feb 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mapleFU commented Feb 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mapleFU commented Feb 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kou commented Feb 12, 2023

Uh oh!

mapleFU commented Feb 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mapleFU commented Feb 10, 2023 •

edited by github-actions bot

Loading

mapleFU commented Feb 10, 2023 •

edited

Loading

mapleFU commented Feb 12, 2023 •

edited

Loading

mapleFU commented Feb 12, 2023 •

edited

Loading

mapleFU commented Feb 12, 2023 •

edited

Loading