You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2016/10/21 23:28:40 UTC
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Tim Armstrong has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4790
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Add all build targets to CMake and speed up builds
Use CMake's dependency resolution always instead of serial execution of
targets via shell scripts. This improves parallelism by building fe,
be, and other targets at the same time and avoid some overhead from
invoking "make" multiple times. This reduces the time taken for
an incremental compilation of fe and be from 56s to 24s with this
command:
./buildall.sh -debug -noclean -notests -skiptests -ninja
Also use Impala-lzo's build script. This depends on the IMPALA-4277
fixes to the Impala-lzo build script.
Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
---
M CMakeLists.txt
M be/src/benchmarks/CMakeLists.txt
M be/src/service/CMakeLists.txt
M bin/make_impala.sh
M buildall.sh
M ext-data-source/CMakeLists.txt
M fe/CMakeLists.txt
7 files changed, 74 insertions(+), 42 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4790/1
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Jim Apple, Alex Behm,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4790
to look at the new patch set (#4).
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Add all build targets to CMake and speed up builds
Use CMake's dependency resolution always instead of serial execution of
targets via shell scripts. This improves parallelism by building fe,
be, and other targets at the same time and avoid some overhead from
invoking "make" multiple times. This reduces the time taken for
an incremental compilation of fe and be from 56s to 24s with this
command:
./buildall.sh -debug -noclean -notests -skiptests -ninja
Also use Impala-lzo's build script. This depends on the IMPALA-4277
fixes to the Impala-lzo build script.
Log directory creation is also moved from impala-config.sh to
buildall.sh. This means that impala-config.sh has no side-effects and
can be run concurrently with no issues.
Also make sure that "make" builds all the same artifacts as buildall.sh
when run with no args.
Testing:
Ran a jenkins core job, also experimented locally. Ran a jenkins core
job with distcc disabled - this exposed some concurrency bugs where
impala-config.sh fails if run concurrently.
Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
---
M CMakeLists.txt
M be/src/benchmarks/CMakeLists.txt
M be/src/service/CMakeLists.txt
M bin/impala-config.sh
M bin/make_impala.sh
M buildall.sh
M ext-data-source/CMakeLists.txt
M fe/CMakeLists.txt
8 files changed, 85 insertions(+), 66 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4790/4
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has uploaded a new patch set (#2).
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Add all build targets to CMake and speed up builds
Use CMake's dependency resolution always instead of serial execution of
targets via shell scripts. This improves parallelism by building fe,
be, and other targets at the same time and avoid some overhead from
invoking "make" multiple times. This reduces the time taken for
an incremental compilation of fe and be from 56s to 24s with this
command:
./buildall.sh -debug -noclean -notests -skiptests -ninja
Also use Impala-lzo's build script. This depends on the IMPALA-4277
fixes to the Impala-lzo build script.
Also make sure that "make" builds all the same artifacts as buildall.sh
when run with no args.
Testing:
Ran a jenkins core job, also experimented locally.
Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
---
M CMakeLists.txt
M be/src/benchmarks/CMakeLists.txt
M be/src/service/CMakeLists.txt
M bin/make_impala.sh
M buildall.sh
M ext-data-source/CMakeLists.txt
M fe/CMakeLists.txt
7 files changed, 65 insertions(+), 52 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4790/2
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 5:
There were a couple of races exposed by running builds with different settings:
* The shell tarball must depend on thrift-deps
* impala-config.sh had side-effects that caused problems if run concurrently
I've included fixes for both in the latest patch
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 6:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/117/
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 2: Code-Review+1
Not +2 because I think it could use a review from someone who knows Cmake much better than I do
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 6:
I'll give that a try - thanks for the tip.
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 7: Code-Review+2
rebase
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 2:
(1 comment)
Have you tried this on a branch that still has thirdparty, e.g., packaging?
http://gerrit.cloudera.org:8080/#/c/4790/2/buildall.sh
File buildall.sh:
Line 314: "$IMPALA_HOME/bin/make_impala.sh" ${MAKE_IMPALA_ARGS}
for my understanding: ext-data-source is build because fe depends on it right?
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 6: Code-Review+2
Rebase
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 3:
The packaging build also helps validate it on multiple operating systems and verifies it's producing the same artifacts as before, so will give us some additional confidence.
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 3:
Confirmed I was able to build on all of our supported platforms with the change. Will wait until our builds stabilise before merging.
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 3: Code-Review+1
Rebase
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 7:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/118/
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 7:
> Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/120/
Sorry, that was me :-(
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 1:
(5 comments)
Have you tested this in a clean environment, maybe with Docker or Jenkins?
http://gerrit.cloudera.org:8080/#/c/4790/1/bin/make_impala.sh
File bin/make_impala.sh:
Line 90: echo "[-fe_only] : Builds fe only."
The reason for this flag is so that make_impala.sh still builds the backend if you pass it no args?
If so, what would be the consequences of breaking compatibility and requiring -be if the user wants to build the backend?
PS1, Line 92: ."
cand benchmarks
PS1, Line 96: like
What else?
PS1, Line 174: exit
This line can be omitted
http://gerrit.cloudera.org:8080/#/c/4790/1/buildall.sh
File buildall.sh:
PS1, Line 305: all
Should the meaning of "all" be put into make_impala.sh?
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/4790/1/bin/make_impala.sh
File bin/make_impala.sh:
Line 90: echo "[-fe_only] : Builds fe only."
> The reason for this flag is so that make_impala.sh still builds the backend
A lot of people depend on make_impala.sh only building the backend unfortunately (I know of several jenkins jobs). Doesn't seem worth the pain, especially since buildall.sh already was a standard way to build everything.
PS1, Line 92: ."
> cand benchmarks
Done
PS1, Line 96: like
> What else?
There used to be a test tarball. We might want to produce a source tarball at some point. Mainly just didn't want to add a new flag every time we added some new build artifact.
PS1, Line 174: exit
> This line can be omitted
Done
http://gerrit.cloudera.org:8080/#/c/4790/1/buildall.sh
File buildall.sh:
PS1, Line 305: all
> Should the meaning of "all" be put into make_impala.sh?
I think it makes more sense to do that at the CMake level. It already builds most of these things when run with no args, but I added in the remaining ones that were not part of ALL (cscope and the tarballs).
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 3: Code-Review+2
lgtm if that remaining test succeeds
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Add all build targets to CMake and speed up builds
Use CMake's dependency resolution always instead of serial execution of
targets via shell scripts. This improves parallelism by building fe,
be, and other targets at the same time and avoid some overhead from
invoking "make" multiple times. This reduces the time taken for
an incremental compilation of fe and be from 56s to 24s with this
command:
./buildall.sh -debug -noclean -notests -skiptests -ninja
Also use Impala-lzo's build script. This depends on the IMPALA-4277
fixes to the Impala-lzo build script.
Log directory creation is also moved from impala-config.sh to
buildall.sh. This means that impala-config.sh has no side-effects and
can be run concurrently with no issues.
Also make sure that "make" builds all the same artifacts as buildall.sh
when run with no args.
Testing:
Ran a jenkins core job, also experimented locally. Ran a jenkins core
job with distcc disabled - this exposed some concurrency bugs where
impala-config.sh fails if run concurrently.
Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Reviewed-on: http://gerrit.cloudera.org:8080/4790
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M CMakeLists.txt
M be/src/benchmarks/CMakeLists.txt
M be/src/service/CMakeLists.txt
M bin/impala-config.sh
M bin/make_impala.sh
M buildall.sh
M ext-data-source/CMakeLists.txt
M fe/CMakeLists.txt
8 files changed, 85 insertions(+), 66 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Tim Armstrong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 7:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/120/
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 5:
> I'm holding off on merging this until builds stabilise a little
> more, since it's not a high priority to get in immediately.
You might want to rebase and try running a GVM dry-run in the public Jenkins. Dry-runs will run all the tests but won't +1 or submit this job.
One of the tests that gets run is bootstrap_build.sh. Another runs with a variety of build flags (asan, so, release, and so on). You can check if these break and if they get any faster than usual.
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 5:
I'm holding off on merging this until builds stabilise a little more, since it's not a high priority to get in immediately.
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] Add all build targets to CMake and speed up builds
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: Add all build targets to CMake and speed up builds
......................................................................
Patch Set 2:
(1 comment)
I'll run a packaging test build
http://gerrit.cloudera.org:8080/#/c/4790/2/buildall.sh
File buildall.sh:
Line 314: "$IMPALA_HOME/bin/make_impala.sh" ${MAKE_IMPALA_ARGS}
> for my understanding: ext-data-source is build because fe depends on it rig
Exactly
--
To view, visit http://gerrit.cloudera.org:8080/4790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23617adf13bdeb034c24f6bba14b5ae480e8dd26
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes