Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21541 )
Change subject: IMPALA-12906: Incorporate scan range information into the tuple cache key ...................................................................... Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/21541/7/be/src/scheduling/scheduler.cc File be/src/scheduling/scheduler.cc: http://gerrit.cloudera.org:8080/#/c/21541/7/be/src/scheduling/scheduler.cc@593 PS7, Line 593: if (a_weight != b_weight) { > nit: Could be a one-liner. As could the assignment above. Done http://gerrit.cloudera.org:8080/#/c/21541/7/be/src/scheduling/scheduler.cc@603 PS7, Line 603: if (a_hdfs_split.mtime() != b_hdfs_split.mtime()) { > If these get more expensive to compute, do we really want to repeat them fo In terms of generated code, for const accessor methods on const references, the compiler will optimize it properly. Using local variables won't change the generated code here. If we do more expensive things, then we should be careful and use local variables. Something like ScanRangeWeight() should get optimized properly, but I'm being cautious there. If there is a better style, I'm open to it. I didn't find local variables to help much. -- To view, visit http://gerrit.cloudera.org:8080/21541 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibe298fff0f644ce931a2aa934ebb98f69aab9d34 Gerrit-Change-Number: 21541 Gerrit-PatchSet: 7 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Mon, 26 Aug 2024 22:48:07 +0000 Gerrit-HasComments: Yes
