You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2019/07/02 00:02:43 UTC

[Impala-ASF-CR] build: use thin static archives

Todd Lipcon has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13775


Change subject: build: use thin static archives
......................................................................

build: use thin static archives

This enables thin static archives for our internal libraries during the
build. This makes linking much faster since the static archives just
point to object files instead of copying them.

Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
---
M CMakeLists.txt
1 file changed, 9 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13775/1
-- 
To view, visit http://gerrit.cloudera.org:8080/13775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
Gerrit-Change-Number: 13775
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] build: use thin static archives

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/13775 )

Change subject: build: use thin static archives
......................................................................


Patch Set 1:

I tried this out and I couldn't tell a difference when doing buildall.sh -skiptests. Can you include a measurement of what got faster?


-- 
To view, visit http://gerrit.cloudera.org:8080/13775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
Gerrit-Change-Number: 13775
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 02 Jul 2019 18:48:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] build: use thin static archives

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13775 )

Change subject: build: use thin static archives
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3799/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/13775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
Gerrit-Change-Number: 13775
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 02 Jul 2019 00:42:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] build: use thin static archives

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13775 )

Change subject: build: use thin static archives
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/13775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
Gerrit-Change-Number: 13775
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 02 Jul 2019 21:31:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] build: use thin static archives

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13775 )

Change subject: build: use thin static archives
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3808/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/13775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
Gerrit-Change-Number: 13775
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 02 Jul 2019 21:41:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] build: use thin static archives

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13775 )

Change subject: build: use thin static archives
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4577/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/13775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
Gerrit-Change-Number: 13775
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 02 Jul 2019 21:31:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] build: use thin static archives

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Joe McDonnell, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13775

to look at the new patch set (#2).

Change subject: build: use thin static archives
......................................................................

build: use thin static archives

This enables thin static archives for our internal libraries during the
build. This makes linking much faster since the static archives just
point to object files instead of copying them.

This reduces the size of intermediate '.a' files for my debug build from
about 1.4GB to 58MB.

Incremental rebuilds are slightly sped up, though maybe in the realm of
noise (likely the performance improvement depends on how much RAM is
available for buffer caching the IO):

Without patch incremental build of catalogd after modifying CatalogObjects.thrift:
real    0m53.433s
user    7m6.400s
sys     1m21.610s

With patch:
real    0m44.772s
user    7m6.632s
sys     1m16.870s

Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
---
M CMakeLists.txt
1 file changed, 9 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13775/2
-- 
To view, visit http://gerrit.cloudera.org:8080/13775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
Gerrit-Change-Number: 13775
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] build: use thin static archives

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13775 )

Change subject: build: use thin static archives
......................................................................

build: use thin static archives

This enables thin static archives for our internal libraries during the
build. This makes linking much faster since the static archives just
point to object files instead of copying them.

This reduces the size of intermediate '.a' files for my debug build from
about 1.4GB to 58MB.

Incremental rebuilds are slightly sped up, though maybe in the realm of
noise (likely the performance improvement depends on how much RAM is
available for buffer caching the IO):

Without patch incremental build of catalogd after modifying CatalogObjects.thrift:
real    0m53.433s
user    7m6.400s
sys     1m21.610s

With patch:
real    0m44.772s
user    7m6.632s
sys     1m16.870s

Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
Reviewed-on: http://gerrit.cloudera.org:8080/13775
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M CMakeLists.txt
1 file changed, 9 insertions(+), 0 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/13775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
Gerrit-Change-Number: 13775
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] build: use thin static archives

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13775 )

Change subject: build: use thin static archives
......................................................................


Patch Set 3: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/13775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
Gerrit-Change-Number: 13775
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 03 Jul 2019 03:08:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] build: use thin static archives

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13775 )

Change subject: build: use thin static archives
......................................................................


Patch Set 1:

> Patch Set 1:
> 
> I tried this out and I couldn't tell a difference when doing buildall.sh -skiptests. Can you include a measurement of what got faster?

hmm, it should make the generated .a files much smaller and thus link faster. Let me run some tests, maybe I put this in the wrong spot in the CMakeLists.txt file (I copied it from Kudu where we saw a speedup but will admit I didn't verify it in Impala)


-- 
To view, visit http://gerrit.cloudera.org:8080/13775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
Gerrit-Change-Number: 13775
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 02 Jul 2019 19:17:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] build: use thin static archives

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/13775 )

Change subject: build: use thin static archives
......................................................................


Patch Set 2: Code-Review+2

This makes sense to me.


-- 
To view, visit http://gerrit.cloudera.org:8080/13775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b54b5be8658c951914758c406cca55d4cc1756e
Gerrit-Change-Number: 13775
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 02 Jul 2019 21:19:05 +0000
Gerrit-HasComments: No