Pranay Singh has posted comments on this change. ( http://gerrit.cloudera.org:8080/8215 )
Change subject: IMPALA-5142 EventSequence displays negative elapsed time. ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/8215/5/be/src/util/runtime-profile-counters.h File be/src/util/runtime-profile-counters.h: http://gerrit.cloudera.org:8080/#/c/8215/5/be/src/util/runtime-profile-counters.h@327 PS5, Line 327: [](Event const &event1, Event const &event2) { : return event1.second < event2.second; > rather than creating a tmp eventlist_sorted, you should create a temp for t I'm dropping this logic to call the sort() all the time to make the result more predictable each time this function gets called and also since the event list will be small so sorting won't be that expensive. http://gerrit.cloudera.org:8080/#/c/8215/5/be/src/util/runtime-profile-counters.h@330 PS5, Line 330: == false > our style uses ! rather than == false Since I'm not checking the event list to be sorted so I won't need this anymore. http://gerrit.cloudera.org:8080/#/c/8215/4/be/src/util/runtime-profile-counters.h File be/src/util/runtime-profile-counters.h: http://gerrit.cloudera.org:8080/#/c/8215/4/be/src/util/runtime-profile-counters.h@326 PS4, Line 326: bool eventlist_sorted = std::is_sorted(events_.begin(), events_.end(), : [](Event const &event1, Event const &event2) { : return event1.second < event2.second; : }); : if (eventlist_sorted == false) { : std::sort(events_.begin(), events_.end(), : [](Event const &event1, Event const &event2) { : return event1.second < event2.second; : }); : > the flipside is that this becomes a cliff in the sense that normally it wil Done as suggested kept the functionality simple, removed the logic of having an extra check. -- To view, visit http://gerrit.cloudera.org:8080/8215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c944396d96473b17b453da3e913ffc56680a896 Gerrit-Change-Number: 8215 Gerrit-PatchSet: 5 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Pranay Singh Gerrit-Comment-Date: Wed, 01 Nov 2017 23:01:41 +0000 Gerrit-HasComments: Yes
