On 9/17/19 2:40 PM, Zhengyu Gu wrote:
On 9/17/19 5:41 AM, Alex Kashchenko wrote:
Hi Zhengyu,
On 09/16/2019 07:04 PM, Zhengyu Gu wrote:
Hi Alex,
On 9/9/19 12:29 PM, Alex Kashchenko wrote:
Hi,
Please review the code change required to backport JDK-8196681 to 8u.
Bug: https://bugs.openjdk.java.net/browse/JDK-8196681
Original review thread:
http://mail.openjdk.java.net/pipermail/awt-dev/2018-September/014302.html
11u changeset:
https://hg.openjdk.java.net/jdk-updates/jdk11u-dev/rev/af9ad0ae9039
Patch does not apply cleanly, because std::chrono is used for
timestamps. And this part of C++11 standard is not supported by
VS2010 toolchain that is used for 8u windows builds. The following
change reimplements the same timestamps without std::chrono:
auto getTimeStamp() -> long long {
- using namespace std::chrono;
- auto timeNow =
duration_cast<milliseconds>(steady_clock::now().time_since_epoch());
-
- return timeNow.count();
+ LARGE_INTEGER freqLarge;
+ ::QueryPerformanceFrequency(&freqLarge);
+ long long freq = freqLarge.QuadPart;
+ LARGE_INTEGER counterLarge;
+ ::QueryPerformanceCounter(&counterLarge);
+ long long counter = counterLarge.QuadPart;
+ long long milliDen = 1000;
+ long long whole = (counter / freq) * milliDen;
+ long long part = (counter % freq) * milliDen / freq;
+ return whole + part;
}
Would this whole + part calculation just (counter * milliDen) / freq?
or you are worry about overflow?
Otherwise, looks good to me.
Thanks for the review!
Yes, overflow here doesn't matter for milliseconds, but will happen if
nanoseconds are used.
Okay, a comment will be good.
Added a comment:
+ // prevent possible overflow
+ long long whole = (counter / freq) * milliDen;
+ long long part = (counter % freq) * milliDen / freq;
+ return whole + part;
Full webrev just for the record:
http://cr.openjdk.java.net/~akasko/jdk8u/8196681/webrev.01/
All other changes to original patch, besides
AccessBridgeDebug#getTimeStamp() above, are either path changes or
string literal changes in debug messages.
[...]
--
-Alex
