Opened 10 years ago

Closed 10 years ago

#9429 closed enhancement (fixed)

doh.robot.mouseMove firing rate

Reported by: liucougar Owned by: liucougar
Priority: high Milestone: 1.4
Component: TestFramework Version: 1.3.0
Keywords: Cc: haysmark
Blocked By: Blocking:

Description (last modified by bill)

While trying to run dojox.sketch functional test (dojox/sketch/tests/test_full.html, click the FT button after robot is initialized) in IE7 within a virtualbox environment (guest is windows xp, host is windows xp), I noticed that it is very slow when the mouse is moved on the drawing area.

Looking at DOHRobot.java, in MouseMoveThread::run(), there are following lines:

robot.setAutoDelay(Math.max(duration/100,1));
robot.mouseMove(x1, y1);
int d = 100;

d is basically how many mouse move events it should generate. I think it would be better if that is not hardcoded here. This is my proposed patch:

robot.setAutoDelay(1);
robot.mouseMove(x1, y1);
int d = duration;

set auto delay to 1 (this is different from the original code if duration !=100, but I think this won't affect much). duration will be used as how many times mouseMove events are fired. So in dojox.sketch test file, I can just specify duration=1, a single mouse event is sufficient in this case.

Alternatively, we can introduce another parameter, say firingrate, to doh.robot.mouseMove, which is used as d above (instead of duration).

Mark Hays' comment: Your approach makes sense. Theoretically it also provides better timing for the cases where users move the mouse over a duration that is not a multiple of 100ms.

Attachments (4)

DOHRobot.jar (40.6 KB) - added by haysmark 10 years ago.
1 event / 20 msecs.
DOHRobot.2.jar (40.6 KB) - added by haysmark 10 years ago.
1 event / 100 msecs.
DOHRobot.3.jar (42.4 KB) - added by haysmark 10 years ago.
Jar compiled with corrected math.
9429.patch (2.8 KB) - added by haysmark 10 years ago.
Fixes #9429. Use correct math to move the mouse within the specified duration.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 10 years ago by liucougar

(In [18076]) refs #9429: commit compiled robot jar file provided by Mark Hays

comment:2 Changed 10 years ago by liucougar

Owner: changed from alex to liucougar

comment:3 Changed 10 years ago by liucougar

Resolution: fixed
Status: newclosed

fixed in [18075]

comment:4 Changed 10 years ago by bill

Cc: haysmark added
Description: modified (diff)
Resolution: fixed
Status: closedreopened

This enhancement broke the Tree DnD tests on both FF and IE (on my PC). I think browsers can't keep up with 1000 events / sec. 10 events / sec is probably a reasonable number.

Given that other people likely have tests that call mouseMove() I think we should fix the test harness so the old tests work, rather than changing known tests.

comment:5 Changed 10 years ago by haysmark

Well I'm worried 10/sec is a bit sparse, in Firebug check log events on a node in the html tab and wave your mouse and you will get more like 50 events / sec. So that's 1 event / 20 ms, defaulting to 5 events for 1 mouse move with the default duration, would that be a better baseline for all?

comment:6 Changed 10 years ago by haysmark

OTOH we already were using 100 events in 1.3, so to avoid breaking people, what if we use 1 event for duration <100ms, and 100 events for everything else as before? My concerns about the timing being off were being addressed at times <100ms anyway.

comment:7 Changed 10 years ago by bill

Try 1 event / 20ms and see if the Tree_dnd.html and Tree_dnd_multiParent tests work on IE6 and FF3.... I'm guessing they won't, especially when running in a virtual machine (but currently it's failing even on a native box for me).

Note that before [18075], when writing the Tree tests I needed to put a 1s or 2s delay after the mousemove's to allow things to sync up reliably... which suggests my 10 events / sec number.

Also, I don't think hardcoding to 100 events when duration >= 100ms is a good idea, because that would mean that I'd continue to have to stick in workaround delays into test files.

comment:8 Changed 10 years ago by liucougar

I think 10/sec is a good compromise

if we really feel like it, we can introduce another parameter to doh.robot.mouseMove, fire_interval, which by default is 100ms (so by default it fires 10/sec). If user wants, he can modify fire_interval

I believe that will fix all existing dijit FT we have, while it should not introduce problems for others

Changed 10 years ago by haysmark

Attachment: DOHRobot.jar added

1 event / 20 msecs.

comment:9 Changed 10 years ago by haysmark

Bill, I've attached a robot that uses 1 event / 20 ms. You were right: your robot tests are failing with the trunk, but with this robot, your robot tests all work great for me. I'm hoping you could try this robot on your machine and compare it with your experiences with the 1.3 robot.

Also, try testing a mouseMove with a 20000 msec duration on your box. This would generate 1000 events, as with cougar's robot with a 1000 msec duration, but they are spaced 20 times apart. I am hoping it will be more reasonably CPU intensive.

comment:10 Changed 10 years ago by bill

This is much better, although I think there's still an overload. Tree_Dnd.html works but if I modify the first test in Tree_Dnd.html to remove the mousemove lag "workaround", changing:

}), 2000);	// browser needs time to react to mouseMove events above

to be:

}), 500);	// browser needs time to react to mouseMove events above

then it still fails. (on IE6 on by T60P thinkpad).

Changed 10 years ago by haysmark

Attachment: DOHRobot.2.jar added

1 event / 100 msecs.

comment:11 Changed 10 years ago by haysmark

I attached another robot with the 10 events / sec for you to performance test. I worry it might break people, because people using the default behavior used to get 100 mouse events but now only get 1.

comment:12 Changed 10 years ago by haysmark

The very last test of Tree_DnD fails for me with only 10 events/sec. Somehow when the test is dragging vegetables from the left to foods on the right, the mouse "jumps" over the hit test that enables the user to drop the item.

comment:13 Changed 10 years ago by haysmark

I recently made a new robot commit fixing Safari 4 and Chrome 2, in the process I introduced what I think is a compromise behavior: #events = log(duration). This prevents the pileup of events for long durations, and at the same time keeps the number of events for very short durations down to just a few.

I tried the sketch test and the mouse jumped around like licougar wanted, also the other tests like Tree_dnd work again for me. I was hoping everyone else would try it and see what they think.

comment:14 Changed 10 years ago by bill

Resolution: fixed
Status: reopenedclosed

OK, let's leave it for that way for now then. The delays in Tree_Dnd.html are no longer needed after your chrome 2 changes.

comment:15 Changed 10 years ago by bill

(In [19100]) Delays waiting for mouse-move events no longer necessary after changes to reduce mouse move firing rate, refs #9429.

comment:16 Changed 10 years ago by haysmark

Priority: normalhigh
Resolution: fixed
severity: normalmajor
Status: closedreopened

I discovered a math error in my robot code. For mouse movement, it is creating duration/log(duration) events, which is great for small durations, but for long durations, it is an astronomical number.

Changed 10 years ago by haysmark

Attachment: DOHRobot.3.jar added

Jar compiled with corrected math.

Changed 10 years ago by haysmark

Attachment: 9429.patch added

Fixes #9429. Use correct math to move the mouse within the specified duration.

comment:17 Changed 10 years ago by haysmark

I attached a potential fix, but it will require further testing.

comment:18 Changed 10 years ago by Douglas Hays

Resolution: fixed
Status: reopenedclosed

(In [20778]) Fixes #9429. Changed mouseMove events to fire log(duration) intermediate events as a compromise (slightly choppy over large distances). Distances are small at the start and end to properly trigger DnD code and grow as the distance increases.

Note: See TracTickets for help on using tickets.