Hello Petr,
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,
It supposed to represent the time when the event is put by OS to its
native message queue... But in fact due to timers 'quant effects' this
time might differ by e.g. 15ms for almost simultaneous events (by just
comparing their GetMessageTime()). With adding our calculations with
different timer types those events might even have opposite time order.
and now it's the time when it's sent to java.
Do we really want to care about what time we would like to use: when the
message is queued by OS or when the message is dequeued by OS?
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?
As mentioned above, the same effect we could easily get with the current
implementation: the system timestamps may differ up to 15ms for almost
simultaneous events. But I don't think this have some great impact on
the scenario you've described.
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?
I'm not sure what msg.time you are talking about... As I understood the
MSG* structure we are creating in native handlers is used for referring
to an original OS event later. It seems that the time field is never
used later. Anyways the 'time' field has a concrete meaning in Windows
docs: number of ms since boot.
It seems that the fix might look risky, but I would like to try making
the code cleaner first instead of adding additional fuzzy logic to it
(just for the case I have an initial conservative fix which just makes
the timestamps sequence non-decreasing).
Do you think we may talk offline to be on the same shape?
Thanks for reviewing!
Anton.
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.
