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.

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

Upgrade to blosc-1.21.0 #2422

Merged
merged 2 commits into from Nov 12, 2021
Merged

Upgrade to blosc-1.21.0 #2422

merged 2 commits into from Nov 12, 2021

Conversation

eric-hughes-tiledb
Copy link
Contributor

@eric-hughes-tiledb eric-hughes-tiledb commented Jul 27, 2021

Blosc had a defect with a duplicate synbol _xgetbv() on recent versions of clang; it was fixed in their release 1.20.0. This PR upgrades to the current release 1.21.0.

Unfortunately, a simple upgrade was not feasible. We had been using files from 1.14.x, insofar as I can determine; the actual version does not seem to have been documented. The files we were using were altered from the originals, and altered in ways that did not make transferring a patch forward to the new version reliable. Furthermore, there had been significant work upstream in the interim. Thus this PR differs significantly in details from what came before, though not at all in terms of scope. Modifications outside of blosc were confined to changing the name of a single #include file.


TYPE: BUG
DESC: upgrade to blosc 1.21.0 from 1.14.x

@eric-hughes-tiledb
Copy link
Contributor Author

eric-hughes-tiledb commented Jul 27, 2021

This PR supersedes #2246. It's the same code, but with the sanitizer changes in CMake reverted. It might have reused the previous PR, but starting afresh was much less trouble with version control. Can delete the branch in the other PR when this one merges.

@Shelnutt2
Copy link
Member

@nguyenv as a low-medium priority can you build this branch in the python ecosystem and validate there is no errors/warnings with all the various compiler setups used for wheels?

${SOURCE_DIR}/src/shuffle-generic.c
${SOURCE_DIR}/src/bitshuffle-stub.c
)
try_compile(AVX2_DETECTED ${CMAKE_CURRENT_BINARY_DIR} ${SOURCE_DIR}/cmake/detect-avx2.c)
Copy link
Member

Choose a reason for hiding this comment

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

These try_compiles are not working currently. I'll check it more this week.

@nguyenv
Copy link
Contributor

nguyenv commented Aug 3, 2021

@nguyenv as a low-medium priority can you build this branch in the python ecosystem and validate there is no errors/warnings with all the various compiler setups used for wheels?

Yes, I am currently working on this. It might take a bit because we are running into issues on Windows with the file path length on CMake.

@nguyenv
Copy link
Contributor

nguyenv commented Aug 3, 2021

https://github.com/TileDB-Inc/TileDB-Py/actions/runs/1095516017

All checks pass when building with TileDB-Py. There are warnings, but I'm sure if they are related to the changes on this PR.

@gspowley
Copy link
Member

gspowley commented Nov 10, 2021

The latest commit makes sure the cmake AVX2 check runs and pulls in the blosc AVX2 files. The upgrade to blosc-1.21.0 fixes an issue compiling blosc on ARM.

@Shelnutt2
Copy link
Member

@gspowley can you rebase this branch to fix the doc failure?

@Shelnutt2 Shelnutt2 marked this pull request as ready for review November 12, 2021 02:18
@Shelnutt2 Shelnutt2 merged commit 89ee28a into dev Nov 12, 2021
@Shelnutt2 Shelnutt2 deleted the eh/v2-upgrade-to-blosc-1.21.0 branch November 12, 2021 12:03
github-actions bot pushed a commit that referenced this pull request Nov 29, 2021
* Upgrade to blosc-1.21.0

* update cmake avx2 check and blosc

Co-authored-by: George Powley <george.powley@gmail.com>
Shelnutt2 pushed a commit that referenced this pull request Nov 29, 2021
* Upgrade to blosc-1.21.0

* update cmake avx2 check and blosc

Co-authored-by: George Powley <george.powley@gmail.com>

Co-authored-by: eric-hughes-tiledb <82400964+eric-hughes-tiledb@users.noreply.github.com>
Co-authored-by: George Powley <george.powley@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants