nsStopwatch.cpp: Fix GetProcessTimes() usage

RESOLVED FIXED in Thunderbird 3.3a1

Status

MailNews Core
Backend
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

({fixed1.9.1})

Trunk
Thunderbird 3.3a1
x86
Windows 2000
fixed1.9.1
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed1.9.2a1 & fixed1.9.1b4: Bv1-MC], URL)

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Comment 1

9 years ago
Created attachment 371845 [details] [diff] [review]
(Av1-CC) nsStopwatch.cpp
Attachment #371845 - Flags: superreview?
Attachment #371845 - Flags: review?
(Assignee)

Updated

9 years ago
Attachment #371845 - Flags: superreview?(dmose)
Attachment #371845 - Flags: superreview?
Attachment #371845 - Flags: review?(dmose)
Attachment #371845 - Flags: review?

Updated

9 years ago
Attachment #371845 - Flags: superreview?(dmose)
Attachment #371845 - Flags: superreview+
Attachment #371845 - Flags: review?(dmose)
Attachment #371845 - Flags: review+
Comment on attachment 371845 [details] [diff] [review]
(Av1-CC) nsStopwatch.cpp

r+sr=dmose; thanks for the patch.
(Assignee)

Comment 3

9 years ago
Created attachment 373103 [details] [diff] [review]
(Av1a-CC) nsStopwatch.cpp

Ah, while there, go a step further.
Attachment #371845 - Attachment is obsolete: true
Attachment #373103 - Flags: superreview?(dmose)
Attachment #373103 - Flags: review?(dmose)
(Assignee)

Comment 4

9 years ago
Created attachment 373494 [details] [diff] [review]
(Av1b-CC) nsStopwatch.cpp

Av1a-CC, plus s/TICKS_PER_SECOND/WIN32_TICK_RESOLUTION/
Attachment #373103 - Attachment is obsolete: true
Attachment #373494 - Flags: superreview?(dmose)
Attachment #373494 - Flags: review?(dmose)
Attachment #373103 - Flags: superreview?(dmose)
Attachment #373103 - Flags: review?(dmose)
(Assignee)

Comment 5

9 years ago
Created attachment 373495 [details] [diff] [review]
(Bv1-MC) stopwatch.cpp
[Checkin: Comment 6 & 8]
Attachment #373495 - Flags: superreview?(roc)
Attachment #373495 - Flags: review?(roc)
Attachment #373495 - Flags: superreview?(roc)
Attachment #373495 - Flags: superreview+
Attachment #373495 - Flags: review?(roc)
Attachment #373495 - Flags: review+
(Assignee)

Comment 6

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


http://hg.mozilla.org/mozilla-central/rev/d9a68f43d7d8
Attachment #373495 - Attachment description: (Bv1-MC) stopwatch.cpp → (Bv1-MC) stopwatch.cpp [Checkin: Comment 6]
Attachment #373495 - Flags: approval1.9.1?
Attachment #373495 - Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 373495 [details] [diff] [review]
(Bv1-MC) stopwatch.cpp
[Checkin: Comment 6 & 8]

a191=beltzner
(Assignee)

Comment 8

9 years ago
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
Attachment #373495 - Attachment description: (Bv1-MC) stopwatch.cpp [Checkin: Comment 6] → (Bv1-MC) stopwatch.cpp [Checkin: Comment 6 & 8]
(Assignee)

Updated

9 years ago
Keywords: fixed1.9.1
Whiteboard: [fixed1.9.1b4]
Target Milestone: --- → mozilla1.9.2a1
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!
(Assignee)

Updated

9 years ago
Depends on: 96669
(Assignee)

Updated

9 years ago
Depends on: 457949
(Assignee)

Comment 10

9 years ago
Created attachment 399103 [details] [diff] [review]
(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"... ;->
Attachment #373494 - Attachment is obsolete: true
Attachment #399103 - Flags: superreview?(dmose)
Attachment #399103 - Flags: review?(dmose)
Attachment #373494 - Flags: superreview?(dmose)
Attachment #373494 - Flags: review?(dmose)
(Assignee)

Comment 11

9 years ago
dmose, ping for r+sr.
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).
Attachment #399103 - Flags: superreview?(dmose)
Attachment #399103 - Flags: review?(dmose)
Attachment #399103 - Flags: review?(bugmail)
Comment on attachment 399103 [details] [diff] [review]
(Av1c-CC) nsStopwatch.cpp
[Checked in: Comment 14]

The patch is fine, but badly bitrotted...
Attachment #399103 - Flags: review?(bugmail) → review+
(Assignee)

Comment 14

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

http://hg.mozilla.org/comm-central/rev/12096a34ac2a
Attachment #399103 - Attachment description: (Av1c-CC) nsStopwatch.cpp → (Av1c-CC) nsStopwatch.cpp [Checked in: Comment 14]
(Assignee)

Updated

8 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [fixed1.9.1b4] → [fixed1.9.1b4: Bv1-MC]
Target Milestone: mozilla1.9.2a1 → Thunderbird 3.3a1
(Assignee)

Updated

8 years ago
Whiteboard: [fixed1.9.1b4: Bv1-MC] → [fixed1.9.2a1 & fixed1.9.1b4: Bv1-MC]
(Assignee)

Updated

8 years ago
No longer depends on: 96669
You need to log in before you can comment on or make changes to this bug.