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

ARROW-14039: [C++][Docs] Indicate memory requirements for building #11205

Closed
wants to merge 6 commits into from

Conversation

bkmgit
Copy link
Contributor

@bkmgit bkmgit commented Sep 21, 2021

No description provided.

@github-actions
Copy link

@github-actions
Copy link

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@bkmgit bkmgit changed the title ARROW-14039 [C++][Docs] Indicate memory requirements for building ARROW-14039: [C++][Docs] Indicate memory requirements for building Sep 21, 2021
@@ -124,7 +124,7 @@ argument is omitted then a release build will be produced.
You need to more options to build on Windows. See
:ref:`developers-cpp-windows` for details.

Minimal release build:
Minimal release build (1Gb of RAM for building or more recommended):
Copy link
Contributor

Choose a reason for hiding this comment

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

How did you arrived at these estimates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a discussion on JIRA, estimates arrived at by measuring when doing compilation. It is suggested to just have an estimate of 4GB.

@@ -135,7 +135,7 @@ Minimal release build:
cmake ..
make

Minimal debug build with unit tests:
Minimal debug build with unit tests (4Gb of RAM for building or more recommended):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you are referring to bytes (GB) and not bits (Gb).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

Indicate minimum RAM requirements, Full Ubuntu docker build fails if less than 8GB of RAM. Minimal and minimal debug build estimates based on measurements.
Copy link
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

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

As mentioned in the JIRA I don't love listing 1GB because that's really a specialized machine at this point and you'd be better off cross compiling on a larger system. In other words, if we started requiring 1.2GB of RAM for some minimal builds (maybe a certain version of gcc or clang) then:

  1. I doubt we'd detect that
  2. I wouldn't want to do too much to fix that if there was no obvious fix

However, this message is better than nothing so I won't stand in the way. @pitrou any concerns? Otherwise I'll merge this tomorrow.

@pitrou
Copy link
Member

pitrou commented Nov 2, 2021

Fair enough.

@@ -41,6 +41,9 @@ Building requires:
sufficient. For Windows, at least Visual Studio 2017 is required.
* CMake 3.5 or higher
* On Linux and macOS, either ``make`` or ``ninja`` build utilities
* At least 1GB of RAM for a minimal build, 4GB for a minimal
debug build with tests and 8GB for a full build using
:ref:`docker <docker>`.
Copy link
Member

Choose a reason for hiding this comment

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

One thing that we've run into is that enabling the Unity build will frequently cause more memory to be required. https://issues.apache.org/jira/browse/ARROW-14555 is an example of that (it appears the VM had/was using 4GB when it was OOMkilled building unity). Maybe we should mention here that in ram-constrained environments that's one flag to turn off (ie. CMAKE_UNITY_BUILD=OFF)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is helpful. It seems that about 8GB is what is needed for most options, but that one can often use less. VM's with 1GB are significantly cheaper than VMs with 8GB

@bkmgit
Copy link
Contributor Author

bkmgit commented Nov 22, 2021

@jonkeane @pitrou @westonpace Can this be merged?

Copy link
Member

@jonkeane jonkeane left a comment

Choose a reason for hiding this comment

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

This looks good to me, I'll merge. And thanks for the patience + nudge forward!

@jonkeane jonkeane closed this in 7a9738a Nov 23, 2021
@ursabot
Copy link

ursabot commented Nov 23, 2021

Benchmark runs are scheduled for baseline = 911a833 and contender = 7a9738a. 7a9738a is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.47% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.04% ⬆️0.04%] ursa-thinkcentre-m75q
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True

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.

None yet

6 participants