Skip to content

Conversation

Tishj
Copy link
Contributor

@Tishj Tishj commented Oct 14, 2024

This PR was sparked by work on new compression related functionality, which made me notice that ZDICT_trainFromBuffer_fastCover was missing.

In an attempt to add the definition for that function, I was already bringing in 6+ new files, so I figured I would spin it off in a new PR that updates the entire ZSTD library we vendor to 1.5.6 (and adds the missing function definitions + files)

@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 14, 2024 15:11
@Tishj Tishj marked this pull request as ready for review October 14, 2024 15:12
@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 15, 2024 07:15
@Tishj Tishj marked this pull request as ready for review October 15, 2024 10:07
@hannes
Copy link
Member

hannes commented Oct 16, 2024

Are we sure we want to add the x86 assembly version? I vaguely recall stripping that before...

@Tishj
Copy link
Contributor Author

Tishj commented Oct 16, 2024

Are we sure we want to add the x86 assembly version? I vaguely recall stripping that before...

I think I stripped the assembly optimizations?
Can you point to the file I missed?

@hannes
Copy link
Member

hannes commented Oct 16, 2024

ah right you already removed it never mind, last time I looked I saw the .S file being added

@Tishj
Copy link
Contributor Author

Tishj commented Oct 16, 2024

ah right you already removed it never mind, last time I looked I saw the .S file being added

Yea I initially had it in, but having it listed as source still didn't compile it correctly so I figured it wasn't worth the hassle of supporting properly

@Mytherin Mytherin changed the base branch from main to feature October 16, 2024 11:19
Copy link
Collaborator

@Mytherin Mytherin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! LGTM - one question:

@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 16, 2024 13:23
@Tishj Tishj marked this pull request as ready for review October 16, 2024 18:05
@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 17, 2024 09:13
@Tishj Tishj marked this pull request as ready for review October 17, 2024 09:13
@Mytherin Mytherin merged commit 4d19100 into duckdb:feature Oct 18, 2024
42 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants