You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2024/02/20 20:37:39 UTC
[native-toolchain-CR] IMPALA-12807: Add mold linker
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21043
Change subject: IMPALA-12807: Add mold linker
......................................................................
IMPALA-12807: Add mold linker
This adds the mold linker to the toolchain. Mold is a new
linker that claims to be significantly faster than gold/lld.
Even though it won't be linked into the Impala executable, it
uses C++20 and needs to be build with the toolchain GCC 10+
compiler.
Testing:
- Ran a local build and used mold to build Impala
Change-Id: Ifc1ad3744ea9d0ff7bf111193652a21542d2b2cf
---
M buildall.sh
A source/mold/build.sh
2 files changed, 44 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/43/21043/1
--
To view, visit http://gerrit.cloudera.org:8080/21043
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifc1ad3744ea9d0ff7bf111193652a21542d2b2cf
Gerrit-Change-Number: 21043
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[native-toolchain-CR] IMPALA-12807: Add mold linker
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21043 )
Change subject: IMPALA-12807: Add mold linker
......................................................................
Patch Set 2:
Tested this with a toolchain build and with Impala using this upstream change: https://gerrit.cloudera.org/#/c/21121/
--
To view, visit http://gerrit.cloudera.org:8080/21043
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ad3744ea9d0ff7bf111193652a21542d2b2cf
Gerrit-Change-Number: 21043
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Mar 2024 16:02:24 +0000
Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12807: Add mold linker
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21043 )
Change subject: IMPALA-12807: Add mold linker
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21043/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21043/1//COMMIT_MSG@10
PS1, Line 10: linker that claims to be significantly faster than gold/lld.
> IMPALA-12807 has some numbers for an incremental build+link of impalad and
Ack
--
To view, visit http://gerrit.cloudera.org:8080/21043
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ad3744ea9d0ff7bf111193652a21542d2b2cf
Gerrit-Change-Number: 21043
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 20 Feb 2024 22:47:36 +0000
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-12807: Add mold linker
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21043 )
Change subject: IMPALA-12807: Add mold linker
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21043/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21043/1//COMMIT_MSG@10
PS1, Line 10: linker that claims to be significantly faster than gold/lld.
Have any measurement on how much this impacts our builds?
Any sense of memory overhead? Like when linking all the test binaries in parallel?
--
To view, visit http://gerrit.cloudera.org:8080/21043
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ad3744ea9d0ff7bf111193652a21542d2b2cf
Gerrit-Change-Number: 21043
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 20 Feb 2024 21:36:14 +0000
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-12807: Add mold linker
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21043 )
Change subject: IMPALA-12807: Add mold linker
......................................................................
Patch Set 2: Verified+1
Thanks for the review, going ahead with this (builds pass on x86_64 and ARM)
--
To view, visit http://gerrit.cloudera.org:8080/21043
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ad3744ea9d0ff7bf111193652a21542d2b2cf
Gerrit-Change-Number: 21043
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Mar 2024 19:28:35 +0000
Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12807: Add mold linker
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21043 )
Change subject: IMPALA-12807: Add mold linker
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/21043
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ad3744ea9d0ff7bf111193652a21542d2b2cf
Gerrit-Change-Number: 21043
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Mar 2024 18:30:20 +0000
Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12807: Add mold linker
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21043 )
Change subject: IMPALA-12807: Add mold linker
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21043/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21043/1//COMMIT_MSG@10
PS1, Line 10: linker that claims to be significantly faster than gold/lld.
> Have any measurement on how much this impacts our builds?
IMPALA-12807 has some numbers for an incremental build+link of impalad and it makes a big difference.
From what I see on a Jenkins job on Centos 7, it doesn't really change the runtime of the automated builds very much (e.g. when they are building a lot of backend tests), but it also doesn't regress them. I think link time is not actually a big portion of our from-scratch build. C++ compilation takes up more. I took a look at memory usage during that run, and it looks fine. My guess is that the max memory usage for an individual link task is probably similar, but if it can go more parallel on the CPU, then it can hold the memory for a shorter duration.
My plan is to make this an option a developer can set without changing the default yet. We currently have a "USE_GOLD_LINKER" option that defaults to true. I was going to change that to something like "IMPALA_LINKER" where it could be ld, gold, or mold. The default would stay gold for the initial commit.
--
To view, visit http://gerrit.cloudera.org:8080/21043
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc1ad3744ea9d0ff7bf111193652a21542d2b2cf
Gerrit-Change-Number: 21043
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 20 Feb 2024 22:31:56 +0000
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-12807: Add mold linker
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21043 )
Change subject: IMPALA-12807: Add mold linker
......................................................................
IMPALA-12807: Add mold linker
This adds the mold linker to the toolchain. Mold is a new
linker that claims to be significantly faster than gold/lld.
Even though it won't be linked into the Impala executable, it
uses C++20 and needs to be build with the toolchain GCC 10+
compiler.
Testing:
- Ran a local build and used mold to build Impala
Change-Id: Ifc1ad3744ea9d0ff7bf111193652a21542d2b2cf
Reviewed-on: http://gerrit.cloudera.org:8080/21043
Reviewed-by: Michael Smith <mi...@cloudera.com>
Tested-by: Joe McDonnell <jo...@cloudera.com>
---
M buildall.sh
A source/mold/build.sh
2 files changed, 44 insertions(+), 0 deletions(-)
Approvals:
Michael Smith: Looks good to me, approved
Joe McDonnell: Verified
--
To view, visit http://gerrit.cloudera.org:8080/21043
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifc1ad3744ea9d0ff7bf111193652a21542d2b2cf
Gerrit-Change-Number: 21043
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>