Closed Bug 487605 Opened 11 years ago Closed 9 years ago

nsStopwatch.cpp: Fix GetProcessTimes() usage

Categories

(MailNews Core :: Backend, defect)

x86
Windows 2000
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.3a1

People

(Reporter: sgautherie, Assigned: sgautherie)

References

()

Details

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

Attachments

(2 files, 3 obsolete files)

Attached patch (Av1-CC) nsStopwatch.cpp (obsolete) — Splinter Review
Attachment #371845 - Flags: superreview?
Attachment #371845 - Flags: review?
Attachment #371845 - Flags: superreview?(dmose)
Attachment #371845 - Flags: superreview?
Attachment #371845 - Flags: review?(dmose)
Attachment #371845 - Flags: review?
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.
Attached patch (Av1a-CC) nsStopwatch.cpp (obsolete) — Splinter Review
Ah, while there, go a step further.
Attachment #371845 - Attachment is obsolete: true
Attachment #373103 - Flags: superreview?(dmose)
Attachment #373103 - Flags: review?(dmose)
Attached patch (Av1b-CC) nsStopwatch.cpp (obsolete) — Splinter Review
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)
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+
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
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]
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!
Depends on: 96669
Depends on: 457949
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)
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+
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]
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [fixed1.9.1b4] → [fixed1.9.1b4: Bv1-MC]
Target Milestone: mozilla1.9.2a1 → Thunderbird 3.3a1
Whiteboard: [fixed1.9.1b4: Bv1-MC] → [fixed1.9.2a1 & fixed1.9.1b4: Bv1-MC]
No longer depends on: 96669
You need to log in before you can comment on or make changes to this bug.