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

Use jemalloc for aligned allocation #165

Merged
merged 2 commits into from
Jun 4, 2020
Merged

Use jemalloc for aligned allocation #165

merged 2 commits into from
Jun 4, 2020

Conversation

xhochy
Copy link
Member

@xhochy xhochy commented Jun 3, 2020

This also fixes the OSX build (cc @MarcAntoineSchmidtQC ) and should also improve on allocation times).

Depends on conda-forge/jemalloc-feedstock#18

@xhochy xhochy requested a review from tbenthompson June 3, 2020 17:57
Copy link
Collaborator

@tbenthompson tbenthompson left a comment

Choose a reason for hiding this comment

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

Looks good to me as long as the CI tests pass. One comment, but I don't think it's necessary to do anything if you don't want to.

src/glm_benchmarks/matrix/sandwich/dense-tmpl.cpp Outdated Show resolved Hide resolved
@tbenthompson
Copy link
Collaborator

Looks like the build is failing with

/opt/conda/bin/../lib/gcc/x86_64-conda-linux-gnu/7.5.0/../../../../x86_64-conda-linux-gnu/bin/ld: cannot find -lquadmath

during the installation of glmnet_python.

That doesn't seem related to any of the changes you made. But I'm pretty sure we pinned to a specific glmnet_python commit so I'm confused why you're getting that error.

@xhochy
Copy link
Member Author

xhochy commented Jun 3, 2020

The CI failure could be related to conda-forge/ctng-compilers-feedstock#23

@MarcAntoineSchmidtQC
Copy link
Member

Just want to point out that this doesn't work for me right now. gxx_impl_linux-64=7.5.0=*_6 and gfortran_impl_linux-64=7.5.0=*_6 are, as the name suggest, linux-only packages. No worries if this is still a work in progress.

@xhochy
Copy link
Member Author

xhochy commented Jun 4, 2020

Just want to point out that this doesn't work for me right now. gxx_impl_linux-64=7.5.0=*_6 and gfortran_impl_linux-64=7.5.0=*_6 are, as the name suggest, linux-only packages. No worries if this is still a work in progress.

They were just temporarily added to fix the CI. You don't need them on OSX.

@xhochy
Copy link
Member Author

xhochy commented Jun 4, 2020

Next jemalloc packaging issue: conda-forge/jemalloc-feedstock#19

@xhochy xhochy requested a review from tbenthompson June 4, 2020 16:16
Copy link
Collaborator

@tbenthompson tbenthompson left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks Uwe!

@tbenthompson tbenthompson merged commit d570aa0 into master Jun 4, 2020
@xhochy xhochy deleted the fix-osx-build branch June 8, 2020 10:43
tbenthompson pushed a commit that referenced this pull request Oct 8, 2021
* Use jemalloc for aligned allocation

* Use sized deallocation
tbenthompson pushed a commit that referenced this pull request Oct 8, 2021
* Use jemalloc for aligned allocation

* Use sized deallocation
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