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

GH-41867: [Java] Parallel to build JNI C++ code #41868

Closed
wants to merge 2 commits into from

Conversation

jinchengchenghh
Copy link
Contributor

@jinchengchenghh jinchengchenghh commented May 29, 2024

Rationale for this change

Accelerate the default JNI C++ build process

What changes are included in this PR?

Add -j to cmake build command

Are these changes tested?

Current tests covered.

Are there any user-facing changes?

No

Copy link

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

@vibhatha
Copy link
Collaborator

should we also add some documentation regarding setting a fixed parallelism for Java users (in case of there are concerns on full resource utilization)?

@lidavidm
Copy link
Member

@github-actions crossbow submit -g java

Copy link

Revision: 3c8bda8

Submitted crossbow builds: ursacomputing/crossbow @ actions-10f0171d4b

Task Status
java-jars GitHub Actions
test-conda-python-3.10-spark-v3.5.0 GitHub Actions
test-conda-python-3.11-spark-master GitHub Actions
test-conda-python-3.8-spark-v3.5.0 GitHub Actions
verify-rc-source-java-linux-almalinux-8-amd64 GitHub Actions
verify-rc-source-java-linux-conda-latest-amd64 GitHub Actions
verify-rc-source-java-linux-ubuntu-20.04-amd64 GitHub Actions
verify-rc-source-java-linux-ubuntu-22.04-amd64 GitHub Actions
verify-rc-source-java-macos-amd64 GitHub Actions

@jinchengchenghh
Copy link
Contributor Author

jinchengchenghh commented May 29, 2024

should we also add some documentation regarding setting a fixed parallelism for Java users (in case of there are concerns on full resource utilization)?

Do we need to add mvn -Dparallel=x to support setting a fixed parallelism? Or remember user in document they can build by CMAKE directly if they would not like to utilize resource fully?

@vibhatha
Copy link
Collaborator

mvn -Dparallel=x

I am not sure if this is a standard maven property? But if we can introduce such a variable to the root and document in development.rst, that would be very useful for most of the Java devs just use mvn install approach to build the project.

cc @lidavidm

@jinchengchenghh
Copy link
Contributor Author

No, it will be arrow defined property. Current -j take effect under command mvn generate-resource. Maybe -DCPP_BUILD_PARALLEL is more reasonable.

@lidavidm
Copy link
Member

lidavidm commented Jun 3, 2024

Does CMAKE_PARALLEL_LEVEL not already work?

@lidavidm
Copy link
Member

lidavidm commented Jun 3, 2024

Sorry that should be CMAKE_BUILD_PARALLEL_LEVEL

@vibhatha
Copy link
Collaborator

vibhatha commented Jun 4, 2024

Sorry that should be CMAKE_BUILD_PARALLEL_LEVEL

I haven't actually checked this. @jinchengchenghh could we verify this?

@vibhatha
Copy link
Collaborator

@jinchengchenghh need any help here?

@jinchengchenghh
Copy link
Contributor Author

Sorry that should be CMAKE_BUILD_PARALLEL_LEVEL

It works, but I think the default parallel mode should be parallel build mode.
Java users may not check if cpp code is compiled parallism when they use mvn generate-resource to compile cpp code.

@lidavidm
Copy link
Member

As is though, this forces parallelism which may also be undesirable

@jinchengchenghh
Copy link
Contributor Author

ok, I will close this PR

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.

3 participants