487605 - nsStopwatch.cpp: Fix GetProcessTimes() usage

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Serge Gautherie (:sgautherie)]

See http://msdn.microsoft.com/en-us/library/ms683223(VS.85).aspx

I'll fix both files.

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1-CC) nsStopwatch.cpp

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 371845 [details] [diff] [review]
(Av1-CC) nsStopwatch.cpp

r+sr=dmose; thanks for the patch.

Comment 3

[Serge Gautherie (:sgautherie)]

Attachment: (Av1a-CC) nsStopwatch.cpp

Ah, while there, go a step further.

Comment 4

[Serge Gautherie (:sgautherie)]

Attachment: (Av1b-CC) nsStopwatch.cpp

Av1a-CC, plus s/TICKS_PER_SECOND/WIN32_TICK_RESOLUTION/

Comment 5

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1-MC) stopwatch.cpp [Checkin: Comment 6 & 8]

Comment 6

[Serge Gautherie (:sgautherie)]

Comment on attachment 373495 [details] [diff] [review]
(Bv1-MC) stopwatch.cpp
[Checkin: Comment 6 & 8]


http://hg.mozilla.org/mozilla-central/rev/d9a68f43d7d8

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 373495 [details] [diff] [review]
(Bv1-MC) stopwatch.cpp
[Checkin: Comment 6 & 8]

a191=beltzner

Comment 8

[Serge Gautherie (:sgautherie)]

Comment on attachment 373495 [details] [diff] [review]
(Bv1-MC) stopwatch.cpp
[Checkin: Comment 6 & 8]


http://hg.mozilla.org/releases/mozilla-1.9.1/rev/93d801a6acae

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 373494 [details] [diff] [review]
(Av1b-CC) nsStopwatch.cpp

>+    // ToDo: May want to use NS_ERROR().

Indeed; please make this change now.

>+  return (double) (ftRealTime.ftInt64 - UNIX_EPOCH_IN_FILE_TIME) *
>+                  WIN32_TICK_RESOLUTION;

>+    printf("Error: GetProcessTimes() failed, 0x%lx\n", (int)GetLastError());
>+#endif

>+  return (double) (ftKernel.ftInt64 + ftUser.ftInt64) * WIN32_TICK_RESOLUTION;

Since you're touching these lines already, how about switching from C-style casts (which are effectively reinterpret_casts) to static_casts, which express intent more precisely, and allow the compiler to do a better job of catching mistakes as the code evolves.

Can you elaborate on why it's worth changing from division to multiplication?

Thanks!

Comment 10

[Serge Gautherie (:sgautherie)]

Attachment: (Av1c-CC) nsStopwatch.cpp [Checked in: Comment 14]

Av1b-CC, with comment 9 suggestion(s).

(In reply to comment #9)

Casts: actually, afaict, these 3 explicit casts had no purpose :-|

Multiply: well, now, I would say "be like MICRO_SECONDS_TO_SECONDS_MULT which was added in the meantime"... ;->

Comment 11

[Serge Gautherie (:sgautherie)]

dmose, ping for r+sr.

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 399103 [details] [diff] [review]
(Av1c-CC) nsStopwatch.cpp
[Checked in: Comment 14]

Redirecting review to Andrew as I think gloda's the main user of this (no sr required now).

Comment 13

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 399103 [details] [diff] [review]
(Av1c-CC) nsStopwatch.cpp
[Checked in: Comment 14]

The patch is fine, but badly bitrotted...

Comment 14

[Serge Gautherie (:sgautherie)]

Comment on attachment 399103 [details] [diff] [review]
(Av1c-CC) nsStopwatch.cpp
[Checked in: Comment 14]

http://hg.mozilla.org/comm-central/rev/12096a34ac2a