impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4397,IMPALA-3259: reduce codegen time and memory
Date Wed, 23 Nov 2016 01:44:42 GMT
Hello Michael Ho,

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

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

Change subject: IMPALA-4397,IMPALA-3259: reduce codegen time and memory

IMPALA-4397,IMPALA-3259: reduce codegen time and memory

A handful of fixes to codegen memory usage:
* Delete the IR module when we're done with it (it can be fairly large)
* Track the compiled code size (typically not that large, but it can add
  up if there are many fragments).
* Estimate optimisation memory requirements and track it in the memory
  tracker. This is very crude but much better than not tracking it.

A handful of fixes to improve codegen time/cost, particularly targeted
at compute stats workloads:
* Avoid over-inlining when there are many aggregate functions,
  conjuncts, etc by adding "NoInline" attributes.
* Don't codegen non-grouping merge aggregations. They will only process
  one row per Impala daemon, so codegen is not worth it.
* Make the Hll algorithm more efficient by specialising the hash function
  based on decimal width.

* This doesn't tackle over-inlining of large expr trees, but a similar
  approach will be used there in a follow-on patch.

Compute stats on functional_parquet.widetable_1000_cols goes from 1min+
of codegen to ~ 5s codegen on my machine. Local perf runs of tpc-h
and targeted perf showed no regressions and some moderate improvements

Also did an experiment to understand the perf consequences of disabling
inlining. I manually set CODEGEN_INLINE_EXPRS_THRESHOLD to 0, and ran:

  drop stats tpch_20_parquet.lineitem
  compute stats tpch_20_parquet.lineitem;

There was no difference in time spent in the agg node: 30.7s with
inlining, 30.5s without.

Change-Id: Id10015b49da182cb181a653ac8464b4a18b71091
M be/src/benchmarks/
M be/src/codegen/
M be/src/codegen/
M be/src/codegen/llvm-codegen.h
M be/src/codegen/mcjit-mem-mgr.h
M be/src/exec/
M be/src/exec/
M be/src/exec/
M be/src/exec/exec-node.h
M be/src/exec/
M be/src/exec/
M be/src/exec/
M be/src/exec/
M be/src/exec/
M be/src/exec/partitioned-aggregation-node.h
M be/src/exec/
M be/src/exec/
M be/src/exec/
M be/src/exprs/
M be/src/exprs/anyval-util.h
M be/src/exprs/
M be/src/exprs/expr.h
M be/src/runtime/
M be/src/runtime/
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/planner/
M fe/src/main/java/org/apache/impala/planner/
M testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
M tests/query_test/
29 files changed, 1,479 insertions(+), 145 deletions(-)

  git pull ssh:// refs/changes/56/4956/13
To view, visit
To unsubscribe, visit

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id10015b49da182cb181a653ac8464b4a18b71091
Gerrit-PatchSet: 13
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Silvius Rus <>
Gerrit-Reviewer: Tim Armstrong <>

View raw message