Matthew Jacobs 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? -- 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
