ARROW-3539: [CI/Packaging] Update scripts to build against vendored jemalloc #2779

kszucs · 2018-10-17T13:52:52Z

Actually to not install jemalloc :)

wesm

+1, now I understand =) Thanks

kszucs · 2018-10-17T13:54:19Z

on bionic the build fails with (regardless of this change)

-- extracting...
     src='/build/apache-arrow-0.12.0~dev20181017/cpp/thirdparty/jemalloc/17c897976c60b0e6e4f4a365c751027244dada7a.tar.gz'
     dst='/build/apache-arrow-0.12.0~dev20181017/cpp_build/jemalloc_ep-prefix/src/jemalloc_ep'
-- extracting... [tar xfz]
-- extracting... [analysis]
-- extracting... [rename]
-- extracting... [clean up]
-- extracting... done
read(): Is a directory
make[5]: warning: -jN forced in submake: disabling jobserver mode.
make[5]: *** INTERNAL: readdir: Bad file descriptor.  Stop.
make[5]: Entering directory '/build/apache-arrow-0.12.0~dev20181017/cpp_build/jemalloc_ep-prefix/src/jemalloc_ep'
make[5]: Leaving directory '/build/apache-arrow-0.12.0~dev20181017/cpp_build/jemalloc_ep-prefix/src/jemalloc_ep'

kszucs · 2018-10-17T13:59:39Z

@wesm how can I disable parallel make for jemalloc_ep?

wesm · 2018-10-17T14:00:49Z

See 1c85e41

kszucs · 2018-10-17T14:04:14Z

Thanks! If that resolves the buonic build should I introduce a cmake flag for that?

kszucs · 2018-10-17T14:07:48Z

However that's another issue. I'm waiting for crossbow builds: https://github.com/kszucs/crossbow/branches/all?utf8=%E2%9C%93&query=build-335 (bionic will fail).

wesm · 2018-10-17T14:11:34Z

Hm, do we understand the root cause of the parallel build failure? That seems odd

kszucs · 2018-10-17T15:07:08Z

Not yet, but there was a similar issue #2245

cc @kou

kou · 2018-10-18T04:28:35Z

Using make -j in sub-make is fragile. And it's useless when we use -j in top-level make. So we should avoid it.

How about the following? We can use parallel build when we use Ninja CMake generator.

diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index 15ff5ec8..7540e782 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -309,6 +309,12 @@ if ("${MAKE}" STREQUAL "")
     endif()
 endif()
 
+if (${CMAKE_GENERATOR} MATCHES "Makefiles")
+  set(MAKE_BUILD_ARGS "")
+else()
+  set(MAKE_BUILD_ARGS "-j")
+endif()
+
 # ----------------------------------------------------------------------
 # Find pthreads
 
@@ -736,9 +742,9 @@ if (ARROW_JEMALLOC)
     CONFIGURE_COMMAND ./autogen.sh "--prefix=${JEMALLOC_PREFIX}" "--with-jemalloc-prefix=je_arrow_" "--with-private-namespace=je_arrow_private_" "--disable-tls"
     ${EP_LOG_OPTIONS}
     BUILD_IN_SOURCE 1
-    BUILD_COMMAND ${MAKE} -j
+    BUILD_COMMAND ${MAKE} ${MAKE_BUILD_ARGS}
     BUILD_BYPRODUCTS "${JEMALLOC_STATIC_LIB}" "${JEMALLOC_SHARED_LIB}"
-    INSTALL_COMMAND ${MAKE} -j1 install)
+    INSTALL_COMMAND ${MAKE} install)
 
   # Don't use the include directory directly so that we can point to a path
   # that is unique to our codebase.

kszucs · 2018-10-18T06:27:05Z

@kou Doesn't jinja do parallel builds by default?

kou · 2018-10-18T06:37:54Z

ninja? ninja does parallel builds by default.

kszucs · 2018-10-18T07:05:02Z

Yes ninja :) Then do We need set(MAKE_BUILD_ARGS "-j")?
So I suppose just the removal of -j argument will work fine.

kou · 2018-10-18T07:15:12Z

Then do We need set(MAKE_BUILD_ARGS "-j")?

Yes. Ninja users like fast build. So we should use -j for Ninja users. The patch enables -j all users except Make users.

kszucs · 2018-10-18T07:21:38Z

I mean ninja -j fails unless We provide the threadnum e.g. ninja -j4, but I think it's not required due to ninja paralellizes by default. So I propose to just remove the -j flag. Do I miss something?

kou · 2018-10-18T07:37:13Z

Yes. ninja requires the maximum number of jobs.

But make doesn't require the maximum number of jobs. If we omit the maximum number of jobs, make doesn't limit the concurrent number of jobs. It causes a problem when we have many source codes. If we process many source codes at once, system resources (normally, CPU and I/O) are conflicted. It reduces build performance. jemalloc only have 30 .c files. It's not very small but it will not use all system resources. It may be better that we use -j4 or -j8 for safety.

kou · 2018-10-19T00:50:28Z

cpp/cmake_modules/ThirdpartyToolchain.cmake

+if (${CMAKE_GENERATOR} MATCHES "Makefiles")
+    set(MAKE_BUILD_ARGS "")
+else()
+    # limit the maximum number of jobs for ninja


You may misunderstand. This argument is used by make not ninja.

Oh I see, so the problem was using make -j instead make -jN, I'll update it.

@kou No, I don't fully understand :)
My conclusion from your reasoning and patch was to not parallelize make at all, and parallelize but limit the number of jobs for ninja -j4.

BTW The build is passing with the current changeset.

kou · 2018-10-19T00:51:07Z

The CI failure is unrelated.
I restarted the failed task.

kou

+1
I'll merge this.

kou · 2018-10-20T23:14:27Z

@kszucs This still have WIP label. Do you have more works for this?

kszucs · 2018-10-21T07:46:14Z

No, removed it.

xhochy

+1, LGTM

On ARROW-6437 (#7928) we saw occasional "File exists" errors on `jemalloc_ep` on macOS. Googling the error message led back to ARROW-739 (#456), which fixed this before by forcing install with `-j1`. ARROW-3492 later made it so jemalloc would build (but not install) in parallel. Then ARROW-3539 (#2779) was addressing a problem with that change and, along with fixing the build parallelization issue, unfortunately reverted the original `make -j1 install` fix. This patch restores the fix from ARROW-739. Closes #7995 from nealrichardson/jemalloc-install Authored-by: Neal Richardson <neal.p.richardson@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

On ARROW-6437 (apache#7928) we saw occasional "File exists" errors on `jemalloc_ep` on macOS. Googling the error message led back to ARROW-739 (apache#456), which fixed this before by forcing install with `-j1`. ARROW-3492 later made it so jemalloc would build (but not install) in parallel. Then ARROW-3539 (apache#2779) was addressing a problem with that change and, along with fixing the build parallelization issue, unfortunately reverted the original `make -j1 install` fix. This patch restores the fix from ARROW-739. Closes apache#7995 from nealrichardson/jemalloc-install Authored-by: Neal Richardson <neal.p.richardson@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

remove jemalloc installations

92899bf

kszucs added the WIP PR is work in progress label Oct 17, 2018

wesm approved these changes Oct 17, 2018

View reviewed changes

make build args

016575c

kou reviewed Oct 19, 2018

View reviewed changes

kou approved these changes Oct 20, 2018

View reviewed changes

kszucs removed the WIP PR is work in progress label Oct 21, 2018

xhochy approved these changes Oct 21, 2018

View reviewed changes

xhochy closed this in de5d7de Oct 21, 2018

This was referenced Aug 18, 2020

ARROW-6437: [R] Add AWS SDK to system dependencies for macOS and Windows #7928

Closed

ARROW-9789: [C++] Don't install jemalloc in parallel #7995

Closed

This was referenced Oct 21, 2018

[CI/Packaging] Update scripts to build against vendored jemalloc #19854

Closed

[C++] Don't install jemalloc in parallel #17316

Closed

This was referenced Jan 9, 2024

[R][C++] CMake 3.28.1 -GMake causes jemalloc subbuild to fail #39517

Closed

GH-39517: [C++] Disable parallelism for jemalloc external project #39522

Merged

[C++] Enable parallelism for Make based ExternalProjects #39549

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARROW-3539: [CI/Packaging] Update scripts to build against vendored jemalloc #2779

ARROW-3539: [CI/Packaging] Update scripts to build against vendored jemalloc #2779

kszucs commented Oct 17, 2018 •

edited

wesm left a comment

kszucs commented Oct 17, 2018

kszucs commented Oct 17, 2018

wesm commented Oct 17, 2018

kszucs commented Oct 17, 2018

kszucs commented Oct 17, 2018 •

edited

wesm commented Oct 17, 2018

kszucs commented Oct 17, 2018 •

edited

kou commented Oct 18, 2018 •

edited

kszucs commented Oct 18, 2018

kou commented Oct 18, 2018

kszucs commented Oct 18, 2018 •

edited

kou commented Oct 18, 2018

kszucs commented Oct 18, 2018 •

edited

kou commented Oct 18, 2018

kou Oct 19, 2018

kszucs Oct 19, 2018

kszucs Oct 19, 2018

kou Oct 20, 2018

kou commented Oct 19, 2018

kou left a comment

kou commented Oct 20, 2018

kszucs commented Oct 21, 2018 •

edited

xhochy left a comment

ARROW-3539: [CI/Packaging] Update scripts to build against vendored jemalloc #2779

ARROW-3539: [CI/Packaging] Update scripts to build against vendored jemalloc #2779

Conversation

kszucs commented Oct 17, 2018 • edited

wesm left a comment

Choose a reason for hiding this comment

kszucs commented Oct 17, 2018

kszucs commented Oct 17, 2018

wesm commented Oct 17, 2018

kszucs commented Oct 17, 2018

kszucs commented Oct 17, 2018 • edited

wesm commented Oct 17, 2018

kszucs commented Oct 17, 2018 • edited

kou commented Oct 18, 2018 • edited

kszucs commented Oct 18, 2018

kou commented Oct 18, 2018

kszucs commented Oct 18, 2018 • edited

kou commented Oct 18, 2018

kszucs commented Oct 18, 2018 • edited

kou commented Oct 18, 2018

kou Oct 19, 2018

Choose a reason for hiding this comment

kszucs Oct 19, 2018

Choose a reason for hiding this comment

kszucs Oct 19, 2018

Choose a reason for hiding this comment

kou Oct 20, 2018

Choose a reason for hiding this comment

kou commented Oct 19, 2018

kou left a comment

Choose a reason for hiding this comment

kou commented Oct 20, 2018

kszucs commented Oct 21, 2018 • edited

xhochy left a comment

Choose a reason for hiding this comment

kszucs commented Oct 17, 2018 •

edited

kszucs commented Oct 17, 2018 •

edited

kszucs commented Oct 17, 2018 •

edited

kou commented Oct 18, 2018 •

edited

kszucs commented Oct 18, 2018 •

edited

kszucs commented Oct 18, 2018 •

edited

kszucs commented Oct 21, 2018 •

edited