Sailesh Mukil has posted comments on this change. Change subject: IMPALA-4014: PARALLEL HEADERS ONLY: Introduce query-wide execution state. ......................................................................
Patch Set 1: > > > (1 comment) > > > > @Matt: > > > > The RuntimeState has members that are relatively more frequently > > accessed than QueryState or FInstanceState members. > > So to have good cache temporal locality, we could leave it as a > > separate class. > > > > But to your point, it is a mess. Now that more people are > bringing > > this up, I feel having a RuntimeState per query wouldn't be a bad > > idea. We can move the fragment instance specific stuff to the > > FInstanceState and leave the query specific stuff in > RuntimeState. > > > > The QueryState on the other hand would just contain state > required > > for execution which relatively isn't accessed that frequently. > > > > Thoughts? > > Oh right, now I remember that cache locality had been brought up, > but I think we can probably work around it. (I guess one of the > issues is that there are larger structures, e.g. thrift > query/fragment descriptors? Those we can store on the heap, e.g. > via a scoped_ptr/unique_ptr on the QueryState/FInstanceState.) IMO > the RuntimeState is pretty gross and messes up your nice new > abstractions, so I think it'd be worth us chatting more about. > Perhaps we can chat in person next wk? Sure! Let's chat in person next week. -- To view, visit http://gerrit.cloudera.org:8080/4301 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If58292a5c377660d97e7e7cc0c3122328eba72ed Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-HasComments: No
