impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5844: use a MemPool for expr result allocations
Date Wed, 04 Oct 2017 16:40:51 GMT
Dan Hecht has posted comments on this change. ( )

Change subject: IMPALA-5844: use a MemPool for expr result allocations

Patch Set 12: Code-Review+2

File be/src/exec/exec-node.h:
PS11, Line 379:   Status QueryMaintenance(RuntimeState* state) WARN_UNUSED_RESULT;
> Yeah I think this is another step towards IMPALA-2399 since there's less en
Yup. Mind dropping a TODO: IMPALA-2399 remove this. 
in here to remind people reading the code that this is not the way we want to do things in
the future?

Also, you could list IMPALA-2399 in the commit message since this is a very nice step toward
File be/src/exec/
PS11, Line 600: 
> Probably not
Cool. Alternatively, if it were worth it, then maybe it'd be better for Clear() (or a wrapper)
to do a quick first check e.g. like check total_allocated_bytes_ and bail if that's under
some threshold.  But if Clear() is cheap enough to just run through, let's do that.
File be/src/exec/scanner-context.h:
PS11, Line 337:   /// single-threaded scan node implementation.
> Mentioned the two cases, since the only reason this is this complicated is 
Once we can get rid of the multi-threaded scanner case, this seems worth revisiting. Do you
agree? If so, I think we should add a subtask about it to the overall MT jira.
File be/src/runtime/sorter.h:
PS11, Line 199:   /// MemPool for allocating data structures used by expression evaluators
in the sorter.
              :   MemPool expr_perm_pool_;
              :   /// MemPool for allocations that hold results of expression evaluation in
the sorter.
              :   /// Cleared periodically during sorting to prevent memory accumulating.
              :   MemPool expr_results_pool_;
              :   /// In memory sorter and less-than comparator.
              :   TupleRowComparator compare_less_than_;
> In TopNNode and ExchangeNode the comparator can't be instantiated until Pre
Was also asking about the MemPool (e.g. in ExecNode we have indirection - is that actually
File be/src/udf/udf.h:
PS12, Line 352: Serialize()
and Finalize()? or is it suppose to just say Serialize() is?

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ba5a7542ed90a49a4b5586c040b5985a7d45b61
Gerrit-Change-Number: 8025
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Comment-Date: Wed, 04 Oct 2017 16:40:51 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message