impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3360: Codegen inserting into runtime filters
Date Thu, 21 Sep 2017 17:50:43 GMT
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8029 )

Change subject: IMPALA-3360: Codegen inserting into runtime filters
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8029/3/be/src/exec/filter-context.cc
File be/src/exec/filter-context.cc:

http://gerrit.cloudera.org:8080/#/c/8029/3/be/src/exec/filter-context.cc@225
PS3, Line 225:   Constant* expr_type_arg = codegen->ConstantToGVPtr(
> So I've been playing around with this more, and I have a theory. I noticed 
That sounds plausible. ToNativePtr() makes a stack allocation so it makes sense that it would
cause problems if you return that pointer from the function. Just generating the call and
calling ToNativePtr() inline in this function would work (I think) since the stack allocation
would have the correct lifetime.

Storing the intermediate values to the heap (i.e. result_) I think is the wrong path to go
down since it will inhibit optimisations (the compiler knows that random pointers won't alias
the stack allocation and that the write to the stack allocation won't be visible outside of
the functoin).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79cf23ad92dadaab996a50a2ca07ef9ebe8639bb
Gerrit-Change-Number: 8029
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Matthew Jacobs <mjacobs@apache.org>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Sep 2017 17:50:43 +0000
Gerrit-HasComments: Yes

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