Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201711012017.vA1KHfb8021670@ip-10-146-233-104.ec2.internal>
Date: Wed, 1 Nov 2017 20:17:40 +0000
From: "Dan Hecht (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, reviews@impala.incubator.apache.org
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-5142_EventSequence_displays_negative_elapsed_time=2E=0A?=
In-Reply-To: <gerrit.1507166458000.I8c944396d96473b17b453da3e913ffc56680a896@gerrit.cloudera.org>
References: <gerrit.1507166458000.I8c944396d96473b17b453da3e913ffc56680a896@gerrit.cloudera.org>
Reply-To: dhecht@cloudera.com, impala-cr@cloudera.com, marcelk@gmail.com,
        reviews@impala.incubator.apache.org
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.14.2
Content-Type: multipart/alternative; boundary="8XfEl6NmRPs="; charset=UTF-8
archived-at: Wed, 01 Nov 2017 20:17:49 -0000

--8XfEl6NmRPs=
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Dan Hecht has posted comments on this change=2E ( http://gerrit=2Ecloudera=
=2Eorg:8080/8215 )

Change subject: IMPALA-5142 EventSequence displays nega=
tive elapsed time=2E
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E


Patch Set 4:

(2 comments)

http://gerrit=2Ecloudera=2Eorg:8080/#/=
c/8215/4/be/src/util/runtime-profile-counters=2Eh
File be/src/util/runtime-=
profile-counters=2Eh:

http://gerrit=2Ecloudera=2Eorg:8080/#/c/8215/4/be/sr=
c/util/runtime-profile-counters=2Eh@326
PS4, Line 326:     bool eventlist_s=
orted =3D true;
             :     int64_t prev =3D 0L;
             :     =
for (const Event& event: events_) {
             :       if (event=2Esecond=
 < prev) {
             :         eventlist_sorted =3D false;
             =
:         break;
             :       }
             :       prev =3D event=
=2Esecond;
             :     }
             :  
why bother with this? the =
list is pretty short and this isn't a critical path, so I think it should b=
e fine to always call sort()=2E
(and if you were to do that, you might as w=
ell use std::is_sorted())


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8215/4/=
be/src/util/runtime-profile-counters=2Eh@336
PS4, Line 336: [](Event const =
&event1,
nit: how about breaking the line before the [] instead of in the m=
iddle of the lambda param list?


-- 
To view, visit http://gerrit=2Ecloud=
era=2Eorg:8080/8215
To unsubscribe, visit http://gerrit=2Ecloudera=2Eorg:80=
80/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Messag=
eType: comment
Gerrit-Change-Id: I8c944396d96473b17b453da3e913ffc56680a896
=
Gerrit-Change-Number: 8215
Gerrit-PatchSet: 4
Gerrit-Owner: Pranay Singh
Ge=
rrit-Reviewer: Dan Hecht <dhecht@cloudera=2Ecom>
Gerrit-Reviewer: Pranay Si=
ngh
Gerrit-Comment-Date: Wed, 01 Nov 2017 20:17:40 +0000
Gerrit-HasComments=
: Yes

--8XfEl6NmRPs=--