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

GH-44976: [C++] Enable mimalloc by default, disable jemalloc by default and more #44951

Merged
merged 1 commit into from
Dec 10, 2024
Merged

GH-44976: [C++] Enable mimalloc by default, disable jemalloc by default and more #44951

merged 1 commit into from
Dec 10, 2024

Conversation

pitrou
Copy link
Member

@pitrou pitrou commented Dec 5, 2024 •
edited
Loading

What changes are included in this PR?

  1. Disable jemalloc by default
  2. Enable mimalloc by default
  3. Some files in arrow/compute are only required when ARROW_COMPUTE is enabled, so we needn't compile them by default

Are these changes tested?

Yes, by existing CI tests.

Are there any user-facing changes?

Yes, but nothing compatibility-breaking.

@pitrou
Copy link
Member Author

pitrou commented Dec 5, 2024

Not sure this is really minor, or if we want to create separate issues per individual change (though that may be overkill). Thoughts @kou ?

pitrou
pitrou commented Dec 5, 2024
View reviewed changes
cpp/src/arrow/CMakeLists.txt
Comment on lines +789 to +791
compute/util.cc
compute/util_internal.cc)
Copy link
Member Author

Choose a reason for hiding this comment

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

Those two files may be moved into the row subdir perhaps.

@pitrou
Copy link
Member Author

pitrou commented Dec 5, 2024

@github-actions crossbow submit -g cpp

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Dec 5, 2024
@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@kou
Copy link
Member

kou commented Dec 6, 2024

Could you open at least one issue for this?
This is not MINOR in our definition: https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Dec 6, 2024
@pitrou pitrou changed the title MINOR: [C++] Assorted CMake improvements GH-44976: [C++] Assorted CMake improvements Dec 9, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 9, 2024

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

@pitrou
MINOR: [C++] Assorted CMake improvements
709e86b 
1) Disable jemalloc by default
2) Enable mimalloc by default
3) Disable more compute modules by default
@pitrou
Copy link
Member Author

pitrou commented Dec 9, 2024

@github-actions crossbow submit -g cpp

@github-actions GitHub Actions

This comment was marked as outdated.

Sign in to view
@pitrou
Copy link
Member Author

pitrou commented Dec 9, 2024

@github-actions crossbow submit -g cpp

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Dec 9, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 9, 2024

Revision: 709e86b

Submitted crossbow builds: ursacomputing/crossbow @ actions-ffd8ebc12b

Task Status
example-cpp-minimal-build-static GitHub Actions
example-cpp-minimal-build-static-system-dependency GitHub Actions
example-cpp-tutorial GitHub Actions
test-alpine-linux-cpp GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-valgrind GitHub Actions
test-cuda-cpp-ubuntu-20.04-cuda-11.2.2 GitHub Actions
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1 GitHub Actions
test-debian-12-cpp-amd64 GitHub Actions
test-debian-12-cpp-i386 GitHub Actions
test-fedora-39-cpp GitHub Actions
test-ubuntu-20.04-cpp GitHub Actions
test-ubuntu-20.04-cpp-bundled GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-20 GitHub Actions
test-ubuntu-22.04-cpp-emscripten GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-24.04-cpp GitHub Actions
test-ubuntu-24.04-cpp-bundled-offline GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions
test-ubuntu-24.04-cpp-gcc-14 GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizer GitHub Actions

@pitrou pitrou marked this pull request as ready for review December 9, 2024 15:52
@kou kou changed the title GH-44976: [C++] Assorted CMake improvements GH-44976: [C++] Enable mimalloc by default, disable jemalloc by default and more Dec 9, 2024
kou
kou approved these changes Dec 10, 2024
View reviewed changes
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

@kou kou merged commit b907c5d into apache:main Dec 10, 2024
39 of 40 checks passed
@kou kou removed the awaiting change review Awaiting change review label Dec 10, 2024
@kou kou mentioned this pull request Dec 10, 2024
Closed
@github-actions github-actions bot added the awaiting merge Awaiting merge label Dec 10, 2024
@pitrou pitrou deleted the cmake_fixes branch December 10, 2024 08:01
@jorisvandenbossche
Copy link
Member

The minimal build nightly tests seem to be failing because of this, see eg https://github.com/ursacomputing/crossbow/actions/runs/12279407480/job/34263376592

@pitrou
Copy link
Member Author

pitrou commented Dec 11, 2024

The minimal build nightly tests seem to be failing because of this, see eg https://github.com/ursacomputing/crossbow/actions/runs/12279407480/job/34263376592

Ah, yes, indeed. It should be a quick fix if anyone is available before I am :)

@pitrou pitrou mentioned this pull request Dec 12, 2024
Closed
pitrou added a commit to pitrou/arrow that referenced this pull request Dec 12, 2024
@pitrou
apacheGH-45006: [CI][Python] Fix test_memory failures
42f05c2 
`test_memory.py` has started failing on some builds after apache#44951 was merged
@pitrou pitrou mentioned this pull request Dec 12, 2024
Merged
@pitrou
Copy link
Member Author

pitrou commented Dec 12, 2024

I submitted #45007 to fix the Python CI failures.

pitrou added a commit to pitrou/arrow that referenced this pull request Dec 13, 2024
@pitrou
apacheGH-45006: [CI][Python] Fix test_memory failures
063a8a1 
`test_memory.py` has started failing on some builds after apache#44951 was merged
pitrou added a commit that referenced this pull request Dec 13, 2024
@pitrou
GH-45006: [CI][Python] Fix test_memory failures (#45007)
f9a6eda 
`test_memory.py` has started failing on some builds after #44951 was merged

* GitHub Issue: #45006

Lead-authored-by: Antoine Pitrou <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kou kou kou approved these changes

Assignees
No one assigned
Labels
awaiting merge Awaiting merge Component: C++
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pitrou @kou @jorisvandenbossche