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 2019/06/01 18:08:53 UTC

[Impala-ASF-CR] IMPALA-8578: reduce dependencies on *metrics.h 

Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13491


Change subject: IMPALA-8578: reduce dependencies on *metrics.h
......................................................................

IMPALA-8578: reduce dependencies on *metrics.h

Before this patch there were 100s of compilation units that pulled in
metrics.h and the significant amount of code in that header. It was
painfuly slow to recompile after changes to that file. The patch
reduces that significantly and mostly eliminates transitive inclusions
via other headers.

* Add metric-defs.h with forward declarations needed to have pointers
  to the various classes.
* Update headers to use metric-defs.h and move includes of *metrics.h
  to the .cc files.
* Add includes, etc to fix compilation errors where files depended
  on transitively-included headers from *metrics.h

This shaved about 30s off the build time on Jenkins - about a 4%
speedup.

Testing:
Ran a core build

Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
---
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-server.h
M be/src/exec/exec-node.h
M be/src/exec/hash-table.h
M be/src/exec/hdfs-orc-scanner.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/parquet-column-readers.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/scan-node.cc
M be/src/experiments/data-provider-test.cc
M be/src/experiments/tuple-splitter-test.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-util.cc
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/client-cache.cc
M be/src/runtime/client-cache.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/initial-reservations.cc
M be/src/runtime/io/data-cache.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/handle-cache.h
M be/src/runtime/io/handle-cache.inline.h
M be/src/runtime/io/hdfs-file-reader.cc
M be/src/runtime/io/local-file-reader.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/sorter.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
M be/src/scheduling/request-pool-service.cc
M be/src/scheduling/request-pool-service.h
M be/src/scheduling/scheduler.h
M be/src/service/client-request-state.cc
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/service/session-expiry-test.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/util/collection-metrics.h
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
A be/src/util/metric-defs.h
M be/src/util/metrics.h
61 files changed, 161 insertions(+), 62 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................

IMPALA-8578: part 1: reduce dependencies on *metrics.h

Before this patch there were 100s of compilation units that pulled in
metrics.h and the significant amount of code in that header. It was
painfully slow to recompile after changes to that file. The patch
reduces that significantly and mostly eliminates transitive inclusions
via other headers.

* Add metrics-fwds.h with forward declarations needed to have pointers
  to the various classes.
* Update headers to use metrics-fwds.h and move includes of *metrics.h
  to the .cc files.
* Add includes, etc to fix compilation errors where files depended
  on transitively-included headers from *metrics.h

This shaved about 30s off the build time on Jenkins - about a 4%
speedup.

I didn't end up removing anything from the headers - that is a bit
more work since most of the classes are templatized and need to
be explicitly instantiated in .cc files if functions are not
all defined in the headers.

Testing:
Ran a core build

Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
---
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-server.h
M be/src/exec/exec-node.h
M be/src/exec/hash-table.h
M be/src/exec/hdfs-orc-scanner.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/parquet-column-readers.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/scan-node.cc
M be/src/experiments/data-provider-test.cc
M be/src/experiments/tuple-splitter-test.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-util.cc
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/client-cache.cc
M be/src/runtime/client-cache.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/initial-reservations.cc
M be/src/runtime/io/data-cache.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/handle-cache.h
M be/src/runtime/io/handle-cache.inline.h
M be/src/runtime/io/hdfs-file-reader.cc
M be/src/runtime/io/local-file-reader.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/sorter.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
M be/src/scheduling/request-pool-service.cc
M be/src/scheduling/request-pool-service.h
M be/src/scheduling/scheduler.h
M be/src/service/client-request-state.cc
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impalad-main.cc
M be/src/service/session-expiry-test.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/util/collection-metrics.h
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
A be/src/util/metrics-fwd.h
M be/src/util/metrics.h
60 files changed, 160 insertions(+), 61 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/13491/5
-- 
To view, visit http://gerrit.cloudera.org:8080/13491
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8578: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: reduce dependencies on *metrics.h
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3476/ : 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/13491
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Jun 2019 18:56:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8578: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: reduce dependencies on *metrics.h
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3474/ : 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/13491
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Jun 2019 18:50:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3486/ : 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/13491
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Jun 2019 19:43:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2019 16:32:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2019 21:54:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13491/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13491/4//COMMIT_MSG@11
PS4, Line 11: painfuly
> nit: painfully
Done


http://gerrit.cloudera.org:8080/#/c/13491/4/be/src/rpc/TAcceptQueueServer.h
File be/src/rpc/TAcceptQueueServer.h:

http://gerrit.cloudera.org:8080/#/c/13491/4/be/src/rpc/TAcceptQueueServer.h@78
PS4, Line 78: std::
> It's strange that it compiles without the namespace-qualifier. Turned out w
Yeah I don't think it's a major issue but it's nice to stay consistent about using std:: in headers.


http://gerrit.cloudera.org:8080/#/c/13491/4/be/src/util/metric-defs.h
File be/src/util/metric-defs.h:

http://gerrit.cloudera.org:8080/#/c/13491/4/be/src/util/metric-defs.h@1
PS4, Line 1: // Licensed to the Apache Software Foundation (ASF) under one
> nit: for filename, maybe 'metric-fwd.h', like <iosfwd>?
Went with metrics-fwd.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2019 16:30:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2019 16:32:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................


Patch Set 7:

Carrying verification through rebase.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2019 21:56:09 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8578: reduce dependencies on *metrics.h

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

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

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

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

Change subject: IMPALA-8578: reduce dependencies on *metrics.h
......................................................................

IMPALA-8578: reduce dependencies on *metrics.h

Before this patch there were 100s of compilation units that pulled in
metrics.h and the significant amount of code in that header. It was
painfuly slow to recompile after changes to that file. The patch
reduces that significantly and mostly eliminates transitive inclusions
via other headers.

* Add metric-defs.h with forward declarations needed to have pointers
  to the various classes.
* Update headers to use metric-defs.h and move includes of *metrics.h
  to the .cc files.
* Add includes, etc to fix compilation errors where files depended
  on transitively-included headers from *metrics.h

This shaved about 30s off the build time on Jenkins - about a 4%
speedup.

I didn't end up removing anything from the headers - that is a bit
more work since most of the classes are templatized and need to
be explicitly instantiated in .cc files if functions are not
all defined in the headers.

Testing:
Ran a core build

Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
---
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-server.h
M be/src/exec/exec-node.h
M be/src/exec/hash-table.h
M be/src/exec/hdfs-orc-scanner.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/parquet-column-readers.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/scan-node.cc
M be/src/experiments/data-provider-test.cc
M be/src/experiments/tuple-splitter-test.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-util.cc
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/client-cache.cc
M be/src/runtime/client-cache.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/initial-reservations.cc
M be/src/runtime/io/data-cache.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/handle-cache.h
M be/src/runtime/io/handle-cache.inline.h
M be/src/runtime/io/hdfs-file-reader.cc
M be/src/runtime/io/local-file-reader.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/sorter.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
M be/src/scheduling/request-pool-service.cc
M be/src/scheduling/request-pool-service.h
M be/src/scheduling/scheduler.h
M be/src/service/client-request-state.cc
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/service/session-expiry-test.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/util/collection-metrics.h
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
A be/src/util/metric-defs.h
M be/src/util/metrics.h
61 files changed, 159 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/13491/3
-- 
To view, visit http://gerrit.cloudera.org:8080/13491
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13491 )

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................

IMPALA-8578: part 1: reduce dependencies on *metrics.h

Before this patch there were 100s of compilation units that pulled in
metrics.h and the significant amount of code in that header. It was
painfully slow to recompile after changes to that file. The patch
reduces that significantly and mostly eliminates transitive inclusions
via other headers.

* Add metrics-fwds.h with forward declarations needed to have pointers
  to the various classes.
* Update headers to use metrics-fwds.h and move includes of *metrics.h
  to the .cc files.
* Add includes, etc to fix compilation errors where files depended
  on transitively-included headers from *metrics.h

This shaved about 30s off the build time on Jenkins - about a 4%
speedup.

I didn't end up removing anything from the headers - that is a bit
more work since most of the classes are templatized and need to
be explicitly instantiated in .cc files if functions are not
all defined in the headers.

Testing:
Ran a core build

Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Reviewed-on: http://gerrit.cloudera.org:8080/13491
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Tim Armstrong <ta...@cloudera.com>
---
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-server.h
M be/src/exec/exec-node.h
M be/src/exec/hash-table.h
M be/src/exec/hdfs-orc-scanner.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/parquet-column-readers.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/scan-node.cc
M be/src/experiments/data-provider-test.cc
M be/src/experiments/tuple-splitter-test.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-util.cc
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/client-cache.cc
M be/src/runtime/client-cache.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/initial-reservations.cc
M be/src/runtime/io/data-cache.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/handle-cache.h
M be/src/runtime/io/handle-cache.inline.h
M be/src/runtime/io/hdfs-file-reader.cc
M be/src/runtime/io/local-file-reader.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/sorter.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
M be/src/scheduling/request-pool-service.cc
M be/src/scheduling/request-pool-service.h
M be/src/scheduling/scheduler.h
M be/src/service/client-request-state.cc
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impalad-main.cc
M be/src/service/session-expiry-test.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/util/collection-metrics.h
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
A be/src/util/metrics-fwd.h
M be/src/util/metrics.h
60 files changed, 160 insertions(+), 61 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13491 )

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................


Patch Set 4: Code-Review+1

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13491/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13491/4//COMMIT_MSG@11
PS4, Line 11: painfuly
nit: painfully


http://gerrit.cloudera.org:8080/#/c/13491/4/be/src/rpc/TAcceptQueueServer.h
File be/src/rpc/TAcceptQueueServer.h:

http://gerrit.cloudera.org:8080/#/c/13491/4/be/src/rpc/TAcceptQueueServer.h@78
PS4, Line 78: std::
It's strange that it compiles without the namespace-qualifier. Turned out we have several headers with 'using std::string'. But maybe it's not a huge issue since it's one of the most common types.


http://gerrit.cloudera.org:8080/#/c/13491/4/be/src/util/metric-defs.h
File be/src/util/metric-defs.h:

http://gerrit.cloudera.org:8080/#/c/13491/4/be/src/util/metric-defs.h@1
PS4, Line 1: // Licensed to the Apache Software Foundation (ASF) under one
nit: for filename, maybe 'metric-fwd.h', like <iosfwd>?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2019 12:28:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2019 11:46:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................

IMPALA-8578: part 1: reduce dependencies on *metrics.h

Before this patch there were 100s of compilation units that pulled in
metrics.h and the significant amount of code in that header. It was
painfully slow to recompile after changes to that file. The patch
reduces that significantly and mostly eliminates transitive inclusions
via other headers.

* Add metrics-fwds.h with forward declarations needed to have pointers
  to the various classes.
* Update headers to use metrics-fwds.h and move includes of *metrics.h
  to the .cc files.
* Add includes, etc to fix compilation errors where files depended
  on transitively-included headers from *metrics.h

This shaved about 30s off the build time on Jenkins - about a 4%
speedup.

I didn't end up removing anything from the headers - that is a bit
more work since most of the classes are templatized and need to
be explicitly instantiated in .cc files if functions are not
all defined in the headers.

Testing:
Ran a core build

Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
---
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-server.h
M be/src/exec/exec-node.h
M be/src/exec/hash-table.h
M be/src/exec/hdfs-orc-scanner.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/parquet-column-readers.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/scan-node.cc
M be/src/experiments/data-provider-test.cc
M be/src/experiments/tuple-splitter-test.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-util.cc
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/client-cache.cc
M be/src/runtime/client-cache.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/initial-reservations.cc
M be/src/runtime/io/data-cache.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/handle-cache.h
M be/src/runtime/io/handle-cache.inline.h
M be/src/runtime/io/hdfs-file-reader.cc
M be/src/runtime/io/local-file-reader.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/sorter.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
M be/src/scheduling/request-pool-service.cc
M be/src/scheduling/request-pool-service.h
M be/src/scheduling/scheduler.h
M be/src/service/client-request-state.cc
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impalad-main.cc
M be/src/service/session-expiry-test.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/util/collection-metrics.h
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
A be/src/util/metrics-fwd.h
M be/src/util/metrics.h
60 files changed, 160 insertions(+), 61 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/13491/7
-- 
To view, visit http://gerrit.cloudera.org:8080/13491
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2019 21:56:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2019 16:32:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3499/ : 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/13491
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2019 17:09:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8578: reduce dependencies on *metrics.h

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

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

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

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

Change subject: IMPALA-8578: reduce dependencies on *metrics.h
......................................................................

IMPALA-8578: reduce dependencies on *metrics.h

Before this patch there were 100s of compilation units that pulled in
metrics.h and the significant amount of code in that header. It was
painfuly slow to recompile after changes to that file. The patch
reduces that significantly and mostly eliminates transitive inclusions
via other headers.

* Add metric-defs.h with forward declarations needed to have pointers
  to the various classes.
* Update headers to use metric-defs.h and move includes of *metrics.h
  to the .cc files.
* Add includes, etc to fix compilation errors where files depended
  on transitively-included headers from *metrics.h

This shaved about 30s off the build time on Jenkins - about a 4%
speedup.

I didn't end up removing anything from the headers - that is a bit
more work since most of the classes are templatized and need to
be explicitly instantiated in .cc files if functions are not
all defined in the headers.

Testing:
Ran a core build

Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
---
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-server.h
M be/src/exec/exec-node.h
M be/src/exec/hash-table.h
M be/src/exec/hdfs-orc-scanner.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/parquet-column-readers.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/scan-node.cc
M be/src/experiments/data-provider-test.cc
M be/src/experiments/tuple-splitter-test.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-util.cc
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/client-cache.cc
M be/src/runtime/client-cache.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/initial-reservations.cc
M be/src/runtime/io/data-cache.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/handle-cache.h
M be/src/runtime/io/handle-cache.inline.h
M be/src/runtime/io/hdfs-file-reader.cc
M be/src/runtime/io/local-file-reader.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/sorter.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
M be/src/scheduling/request-pool-service.cc
M be/src/scheduling/request-pool-service.h
M be/src/scheduling/scheduler.h
M be/src/service/client-request-state.cc
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/impalad-main.cc
M be/src/service/session-expiry-test.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/util/collection-metrics.h
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
A be/src/util/metric-defs.h
M be/src/util/metrics.h
61 files changed, 160 insertions(+), 62 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

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

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

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

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................

IMPALA-8578: part 1: reduce dependencies on *metrics.h

Before this patch there were 100s of compilation units that pulled in
metrics.h and the significant amount of code in that header. It was
painfuly slow to recompile after changes to that file. The patch
reduces that significantly and mostly eliminates transitive inclusions
via other headers.

* Add metric-defs.h with forward declarations needed to have pointers
  to the various classes.
* Update headers to use metric-defs.h and move includes of *metrics.h
  to the .cc files.
* Add includes, etc to fix compilation errors where files depended
  on transitively-included headers from *metrics.h

This shaved about 30s off the build time on Jenkins - about a 4%
speedup.

I didn't end up removing anything from the headers - that is a bit
more work since most of the classes are templatized and need to
be explicitly instantiated in .cc files if functions are not
all defined in the headers.

Testing:
Ran a core build

Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
---
M be/src/catalog/catalog-server.cc
M be/src/catalog/catalog-server.h
M be/src/exec/exec-node.h
M be/src/exec/hash-table.h
M be/src/exec/hdfs-orc-scanner.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/parquet-column-readers.cc
M be/src/exec/plan-root-sink.cc
M be/src/exec/scan-node.cc
M be/src/experiments/data-provider-test.cc
M be/src/experiments/tuple-splitter-test.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-util.cc
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/bufferpool/buffer-allocator-test.cc
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/client-cache.cc
M be/src/runtime/client-cache.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/initial-reservations.cc
M be/src/runtime/io/data-cache.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/handle-cache.h
M be/src/runtime/io/handle-cache.inline.h
M be/src/runtime/io/hdfs-file-reader.cc
M be/src/runtime/io/local-file-reader.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M be/src/runtime/krpc-data-stream-mgr.h
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/query-exec-mgr.cc
M be/src/runtime/query-state.cc
M be/src/runtime/sorter.cc
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
M be/src/runtime/tmp-file-mgr.h
M be/src/scheduling/request-pool-service.cc
M be/src/scheduling/request-pool-service.h
M be/src/scheduling/scheduler.h
M be/src/service/client-request-state.cc
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M be/src/service/impalad-main.cc
M be/src/service/session-expiry-test.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore.cc
M be/src/statestore/statestore.h
M be/src/util/collection-metrics.h
M be/src/util/impalad-metrics.cc
M be/src/util/impalad-metrics.h
A be/src/util/metric-defs.h
M be/src/util/metrics.h
60 files changed, 160 insertions(+), 61 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/13491/4
-- 
To view, visit http://gerrit.cloudera.org:8080/13491
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-8578: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: reduce dependencies on *metrics.h
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3475/ : 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/13491
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Jun 2019 18:55:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h

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

Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3503/ : 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/13491
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb
Gerrit-Change-Number: 13491
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Jun 2019 18:41:59 +0000
Gerrit-HasComments: No