Lars Volker has posted comments on this change. Change subject: IMPALA-5487: Fix race in RuntimeProfile::toThrift() ......................................................................
Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7154/1/be/src/util/runtime-profile.cc File be/src/util/runtime-profile.cc: PS1, Line 764: children_.size() > The reserve() shouldn't matter for correctness, but this is wonky. Yes, I left it there since it doesn't change correctness and I assumed in the average case it will still be usefull. I agree that it is confusing. I could either - Remove it - Add a comment and explain why it is here - Move it below where we add the children and where it may matter more I'm in favor of removing it, the number of children seems to be small in general, but if we're concerned about performance we may want to move it down where the children are copied. -- To view, visit http://gerrit.cloudera.org:8080/7154 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7fad4ee2eee1f73e387c1e90a3db265b19b3a0c6 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
