<AWT Dev> [8u] RFR: 8196681: Java Access Bridge logging and debug flags dynamically controlled

[Alex Kashchenko]

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