impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5844: use a MemPool for expr local allocations
Date Thu, 21 Sep 2017 23:53:18 GMT
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8025 )

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


Patch Set 6:

(5 comments)

Had a few high level questions first.

http://gerrit.cloudera.org:8080/#/c/8025/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8025/6//COMMIT_MSG@13
PS6, Line 13: * They are freed in bulk at various points in query execution.
we don't have to do it that way though.


http://gerrit.cloudera.org:8080/#/c/8025/6//COMMIT_MSG@31
PS6, Line 31: Remove FunctionContext::ReallocateLocal()
is that a UDF breaking change?


http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/exec/exec-node.h
File be/src/exec/exec-node.h:

http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/exec/exec-node.h@324
PS6, Line 324: expr-managed 
what does that mean, especially given that it can be used by multiple evaluators? maybe it
helps to explain these in terms of their lifetimes?


http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/exprs/scalar-expr-evaluator.h
File be/src/exprs/scalar-expr-evaluator.h:

http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/exprs/scalar-expr-evaluator.h@74
PS6, Line 74: Evaluator-managed allocations from this
            :   /// evaluator will be from 'expr_mem_pool' while local allocations will be
from
            :   /// 'local_mem_pool'.
that seems pretty tough to understand and distinguish.  what does "evaluator-managed" mean?
 maybe more clear to explain this in terms of the lifetimes?  Oh, I guess that's what you
mean by evaluator-managed -- has the lifetime of the evaluator?


http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/runtime/free-pool.h
File be/src/runtime/free-pool.h:

http://gerrit.cloudera.org:8080/#/c/8025/6/be/src/runtime/free-pool.h@a69
PS6, Line 69: 
why does this go away?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ba5a7542ed90a49a4b5586c040b5985a7d45b61
Gerrit-Change-Number: 8025
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Sep 2017 23:53:18 +0000
Gerrit-HasComments: Yes

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