Make QPCValueToMicroseconds faster in case the value is less than 44 bits

base/time/time_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 
 // Windows Timer Primer
 //
 // A good article:  http://www.ddj.com/windows/184416651
 // A good mozilla bug:  http://bugzilla.mozilla.org/show_bug.cgi?id=363258
 //
@@ skipping 373 matching lines @@
     return RolloverProtectedNow();
   }
 
   int64 GetQPCDriftMicroseconds() {
     if (!IsUsingHighResClock())
       return 0;
     return abs((UnreliableNow() - ReliableNow()) - skew_);
   }
 
   int64 QPCValueToMicroseconds(LONGLONG qpc_value) {
-    if (!ticks_per_second_)

[brianderson, 2014/07/29 23:53:29]

I remember you had an explanation for why this could be removed. Can you explain
again?

[fmeawad, 2014/07/30 00:49:49]

I added that check in the CPUReliablySupportsHighResTime, that is used to set
the function pointer to the correct function.
By moving the check to the setup phase, we guarantee that no calls will have the
ticks_per_second_ = 0

[brianderson, 2014/07/30 00:58:19]

Thanks for the explanation.

I was thinking about asking for a DCHECK to replace the if, but I guess we'll
get a division by zero exception anyway.

-      return 0;
-
-    // Intentionally calculate microseconds in a round about manner to avoid
-    // overflow and precision issues. Think twice before simplifying!
-    int64 whole_seconds = qpc_value / ticks_per_second_;
-    int64 leftover_ticks = qpc_value % ticks_per_second_;
-    int64 microseconds = (whole_seconds * Time::kMicrosecondsPerSecond) +
-                         ((leftover_ticks * Time::kMicrosecondsPerSecond) /
-                          ticks_per_second_);
-    return microseconds;
+    // Time::kMicrosecondsPerSecond has 20 bits, to avoid overflow we do a
+    // simple multiplication then division if the qpc_value has less than 44
+    // bits.
+    if (qpc_value & 0xFFFFF00000000000)

[jar (doing other things), 2014/07/30 23:43:09]

Please unit test at exactly above and below this border.

I *think* this constant might be too big....

should be around 2^63 - 1

...and then a compare, rather than a "&" might get you more mileage.

[jar (doing other things), 2014/07/31 00:01:43]

I think the largest positive 64 bit number is 2^63 - 1
Since you want to multiply by 1,000,000 (number of microseconds per second), to
avoid an overflow, you'll need to avoid being larger than:
(2^63 - 1) / 1E6
That evaluates to: 
9223372036854
in hex that is:
0x8637BD05AF6

Your current test checks to see if the number has any bits in the high end,
which means you're doing a "compare" with:
if (qpc_value >= 0x100000000000)

but the actual test you need is probably:
if (qpc_value >  0x8637BD05AF6)
or
if (qpc_value >= 0x8637BD05AF7)

The unit test should be able to get all this right, and validate correctness
against the "standard" (previous?) code.

WDYT?

[fmeawad, 2014/07/31 18:54:21]

Done.

[fmeawad, 2014/07/31 18:54:21]

Done.

+    {
+      // Intentionally calculate microseconds in a round about manner to avoid
+      // overflow and precision issues. Think twice before simplifying!
+      int64 whole_seconds = qpc_value / ticks_per_second_;
+      int64 leftover_ticks = qpc_value % ticks_per_second_;
+      int64 microseconds = (whole_seconds * Time::kMicrosecondsPerSecond) +
+                           ((leftover_ticks * Time::kMicrosecondsPerSecond) /
+                            ticks_per_second_);
+      return microseconds;
+    }
+    return qpc_value * Time::kMicrosecondsPerSecond / ticks_per_second_;

[brianderson, 2014/07/29 23:53:29]

Would a single double-precision multiplication be faster than an integer
multiply and divide? I think 64-bit integer division takes significantly longer
than a double-precision multiplication on a lot of hardware.

If it's faster, we should be able to apply the same optimization to
leftover_ticks above too.

[fmeawad, 2014/07/30 00:49:49]

I made a quick experiment, and I have found no win. Also for a large sample I
found some loss of percision errors. (1 in 3 million).

[brianderson, 2014/07/30 00:58:19]

Thanks for testing that.

   }
 
  private:
   // Synchronize the QPC clock with GetSystemTimeAsFileTime.
   void InitializeClock() {
     LARGE_INTEGER ticks_per_sec = {0};
     if (!QueryPerformanceFrequency(&ticks_per_sec))
       return;  // Broken, we don't guarantee this function works.
     ticks_per_second_ = ticks_per_sec.QuadPart;
 
@@ skipping 25 matching lines @@
 
 TimeDelta HighResNowWrapper() {
   return GetHighResNowSingleton()->Now();
 }
 
 typedef TimeDelta (*NowFunction)(void);
 NowFunction now_function = RolloverProtectedNow;
 
 bool CPUReliablySupportsHighResTime() {
   base::CPU cpu;
-  if (!cpu.has_non_stop_time_stamp_counter())
+  if (!cpu.has_non_stop_time_stamp_counter() ||
+      !GetHighResNowSingleton()->IsUsingHighResClock())
     return false;
 
   if (IsBuggyAthlon(cpu))
     return false;
 
   return true;
 }
 
 }  // namespace
 
@@ skipping 66 matching lines @@
     return TimeTicks() + TimeDelta::FromMilliseconds(timeGetTime());
   }
 }
 
 // TimeDelta ------------------------------------------------------------------
 
 // static
 TimeDelta TimeDelta::FromQPCValue(LONGLONG qpc_value) {
   return TimeDelta(GetHighResNowSingleton()->QPCValueToMicroseconds(qpc_value));
 }