On 7/18/2014 2:28 PM, anton nashatyrev wrote:
Hello,
in offline discussion with Artem and Petr we decided to further
clean up the code and completely remove TimeHelper functions from
awt_Component.cpp in JDK9.
Could you please review the updated fix:
http://cr.openjdk.java.net/~anashaty/8046495/9/webrev.01/
<http://cr.openjdk.java.net/%7Eanashaty/8046495/9/webrev.01/>
Looks fine to me.
Thanks,
Artem
bug: https://bugs.openjdk.java.net/browse/JDK-8046495
Thank you!
Anton.
On 17.07.2014 13:14, anton nashatyrev wrote:
Hello All,
could please anyone else take a look at the fix?
Thanks!
Anton.
On 02.07.2014 19:37, Petr Pchelko wrote:
Hello, Anton.
Thanks for clarifications and additional testing.
The fix looks good to me too.
With best regards. Petr.
On 02 июля 2014 г., at 19:34, Anton V. Tarasov
<anton.tara...@oracle.com <mailto:anton.tara...@oracle.com>> wrote:
On 02.07.2014 19:28, anton nashatyrev wrote:
Hello, Anton
On 02.07.2014 18:13, Anton V. Tarasov wrote:
On 02.07.2014 11:44, Petr Pchelko wrote:
Hello, Anton.
I'm not sure I have a detailed understanding of what's happening.
Before your fix the timestamp of the event represented the time
when the event was created, and now it's the time when it's sent
to java.
This might be important if some events cause other events to be
issued on the java side.
So suppose the following:
Toolkit thread: InputEvent#1 created - timestamp 0
Toolkit thread: InputEvent#2 created - timestamp 1
Toolkit thread: InputEvent#1 sent - timestamp 2
EDT: InputEvent#1 dispatched - timestamp 3
EDT: FocusEvent created - timestamp 4
Toolkit thread: InputEvent#2 sent - timestamp 5
Before you fix we had the following event order:
InputEvent#1(ts=0), InputEvent#2(ts=1), FocusEvent(ts=4).
But after your fix we will have a different order:
InputEvent#1(ts=2), FocusEvent(ts=4), InputEvent#2(ts=5).
So now we would have troubles, because the Input Event will go to
the new focused component instead of the old one.
Do you think that my arguments are correct? I understand that the
likelihood of such a situation is very low, but for me it looks
possible? Am I missing something?
A timestamp for a FocusEvent is taken from
the_most_recent_KeyEvent_time which is set on EDT when the event
is dispatched. So the fix shouldn't affect it.
Also, in awt_Window.cpp, when a TimedWindowEvent is created it is
passed a timestamp got with TimeHelper::getMessageTimeUTC(). It
appears, that the fix makes key events times even more consistent
with it. So, from the kbd focus perspective, it shouldn't make any
harm.
Anton, could you just please run the following tests, at least:
test/java/awt/Focus/6981400/*
test/java/awt/KeyboardFocusManager/TypeAhead/*
I've tested with the following set:
[closed]/java/awt/event/*
[closed]/java/awt/Focus/*
[closed]java/awt/KeyboardFocusManager/*
: no unexpected failures here.
I've also verified that my regression test which comes with the fix
works fine on Linux and Mac (despite the fix is Win specific).
Great, thanks!
Anton.
Thanks for review!
Anton.
Thanks,
Anton.
Another thing I do not understand is why we were used to use the
complicated formula instead of initializing the msg.time field
with the JVM current time and using it when sending the event?
Wouldn't that resolve both your issue and the issue the original
fix was made for?
I have a couple of comments about the code, but let's postpone
that until we decide on the approach.
Thank you.
With best regards. Petr.
On 01 июля 2014 г., at 21:20, anton nashatyrev
<anton.nashaty...@oracle.com
<mailto:anton.nashaty...@oracle.com>> wrote:
Hello,
could you please review the following fix:
fix: http://cr.openjdk.java.net/~anashaty/8046495/9/webrev.00/
<http://cr.openjdk.java.net/%7Eanashaty/8046495/9/webrev.00/>
bug: https://bugs.openjdk.java.net/browse/JDK-8046495
*Problem:*
On Windows the later InputEvent may have earlier timestamp
(getWhen()), which results in incorrect processing of enqueued
input events and may also potentially be the reason of other
artifacts
*Evaluation*:
On Windows we have some algorithm for calculating input event
timestamp: jdk/src/windows/native/sun/windows/awt_Component.cpp:2100
Shortly the timestamp is calculated by the following formula:
when = JVM_CurrentTimeMillis() - (GetTickCount() -
GetMessageTime())
Where:
JVM_CurrentTimeMillis() - the same as System.currentTimeMillis()
GetTickCount() - Win32 function, current millis from boot time
GetMessageTime() - Win32 function, millis from boot time of
the latest native Message
In theory the formula looks good: we are trying our best to
calculate the actual time of the OS message by taking the
current JVM time (JVM_CurrentTimeMillis) and adjusting it with
the offset (GetTickCount - GetMessageTime) which should indicate
how many milliseconds ago from the current moment (GetTickCount)
the message has been actually issued (GetMessageTime).
In practice due to usage of different system timers by the
JVM_CurrentTimeMillis and GetTickCount their values are not
updated synchronously and we may get an earlier timestamp for
the later event.
*Fix*:
Just to use JVM_CurrentTimeMillis only as events timestamp. On
Mac this is done in exactly the same way:
CPlatformResponder.handleMouse/KeyEvent()
The message time offset calculation has been introduced with the
fix for JDK-4434193
<https://bugs.openjdk.java.net/browse/JDK-4434193> and it seems
that the issue has addressed very similar problem (At times
getWhen()in ActionEvent returns higher value than the
CurrentSystemTime) which has not been completely resolved in fact.
I also didn't find any benefits in using the existing approach:
- all the usages of the TimerHelper are in fact reduced to the
mentioned formula: when = JVM_CurrentTimeMillis() -
(GetTickCount() - GetMessageTime())
- GetMessageTime() always increases (except of the int overflow
moments), thus we couldn't get earlier OS messages after later ones
- TimerHelper::windowsToUTC(DWORD windowsTime) doesn't guarantee
returning the same timestamp across different calls for the same
message time
Thanks!
Anton.
