Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[C++] Thirdparty: Do we need bump zstd to 1.5.6? #40814

Closed
mapleFU opened this issue Mar 27, 2024 · 5 comments
Closed

[C++] Thirdparty: Do we need bump zstd to 1.5.6? #40814

mapleFU opened this issue Mar 27, 2024 · 5 comments
Assignees
@mapleFU
Labels
Component: C++ Type: enhancement
Milestone
16.0.0

Comments

@mapleFU
Copy link
Member

mapleFU commented Mar 27, 2024

Describe the enhancement requested

https://github.com/facebook/zstd/releases/tag/v1.5.6

Zstd releases 1.5.6 . It doesn't include lots of performance improvement. So I wonder should we upgrade it?

Component(s)

C++
@mapleFU
Copy link
Member Author

mapleFU commented Mar 27, 2024

cc @kou @pitrou @felipecrv

@kou
Copy link
Member

kou commented Mar 27, 2024

In general, yes. Incremental upgrades are easier than one big upgrade.

@github-actions github-actions bot mentioned this issue Mar 27, 2024
Merged
@ericsun2
Copy link

On a related topic, we noticed that the Golang implementation of Arrow uses https://github.com/klauspost/compress/tree/master/zstd instead of the native lib (such 1.5.6 above). The pure Go implementation of ZSTD is visibly slower than the native one, especially for modern AVX/SIMD optimization and ARM Graviton CPU.

Is there any consideration to have a feature flag to switch ZSTD lib between pure Go and native shared lib?

Thank you.

@pitrou
Copy link
Member

pitrou commented Mar 27, 2024

@ericsun2 Can you open a new issue for that and categorize it for the Go component? Thank you!

kou pushed a commit that referenced this issue Mar 28, 2024
@mapleFU
GH-40814: [C++] Thirdparty: bump zstd to 1.5.6 (#40837)
b270dcd 
### Rationale for this change

Zstd releases 1.5.6 here: https://github.com/facebook/zstd/releases/tag/v1.5.6

### What changes are included in this PR?

Change default zstd to 1.5.6

### Are these changes tested?

Already has test

### Are there any user-facing changes?

no

* GitHub Issue: #40814

Authored-by: mwish <maplewish117@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 16.0.0 milestone Mar 28, 2024
@kou
Copy link
Member

kou commented Mar 28, 2024

Issue resolved by pull request 40837
#40837

@kou kou closed this as completed Mar 28, 2024
tolleybot pushed a commit to tmct/arrow that referenced this issue May 2, 2024
@mapleFU @tolleybot
apacheGH-40814: [C++] Thirdparty: bump zstd to 1.5.6 (apache#40837)
312fe85 
### Rationale for this change

Zstd releases 1.5.6 here: https://github.com/facebook/zstd/releases/tag/v1.5.6

### What changes are included in this PR?

Change default zstd to 1.5.6

### Are these changes tested?

Already has test

### Are there any user-facing changes?

no

* GitHub Issue: apache#40814

Authored-by: mwish <maplewish117@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
vibhatha pushed a commit to vibhatha/arrow that referenced this issue May 25, 2024
@mapleFU @vibhatha
apacheGH-40814: [C++] Thirdparty: bump zstd to 1.5.6 (apache#40837)
6bfae6b 
### Rationale for this change

Zstd releases 1.5.6 here: https://github.com/facebook/zstd/releases/tag/v1.5.6

### What changes are included in this PR?

Change default zstd to 1.5.6

### Are these changes tested?

Already has test

### Are there any user-facing changes?

no

* GitHub Issue: apache#40814

Authored-by: mwish <maplewish117@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mapleFU mapleFU

Labels
Component: C++ Type: enhancement
Projects
None yet
Milestone
16.0.0
Development

No branches or pull requests
4 participants
@kou @pitrou @ericsun2 @mapleFU