Fixing the bug didn't change the performance noticably:
Bug fix is:
quickRunCount = (int) (((TIME_SLICE - 1) * runCount) / elapsedMillis);
becomes:
quickRunCount = (int) (((TIME_SLICE - 1) * runCount) / elapsedMillis) -
runCount;
On Wed, Jan 14, 2015 at 5:37 PM, Richard Wallis <rdwal...@gmail.com> wrote:
> Ah second implementation has a bug again that makes it much faster than it
> should be I'll post the fix and the actual speed up, if any, shortly.
>
> On Wed, Jan 14, 2015 at 5:34 PM, Richard Wallis <rdwal...@gmail.com>
> wrote:
>
>> On a side note the Scheduler implentation needs to be rewritten to make
>> it easier to provide alternative schedulers while letting those alternative
>> schedulers use as much of the core code as possible.
>>
>> It would be easier to make these patches if runRepeatingTasks was
>> protected and there was a Scheduler interface instead of an abstract class
>> with so many nested classes.
>>
>>
>> On Wed, Jan 14, 2015 at 5:25 PM, Richard Wallis <rdwal...@gmail.com>
>> wrote:
>>
>>> Also, your approach is assuming that all tasks in the queue are "the
>>>> same task" that needs repeating. But what if I schedule 2 incremental
>>>> tasks?
>>>
>>>
>>> Loop count code takes this into account and only runs if the length of
>>> the task queue == 1.
>>>
>>> The second approach where you divide the TIME_SLICE by elapsed time and
>>> then multiply by how many tasks have run will work with multiple tasks at a
>>> time. It's dangerous in the current implementation because the faster job
>>> can die before a slower job messing up the estimate. But it can be made
>>> safe
>>>
>>> On Wed, Jan 14, 2015 at 5:14 PM, Thomas Broyer <t.bro...@gmail.com>
>>> wrote:
>>>
>>>> Also, your approach is assuming that all tasks in the queue are "the
>>>> same task" that needs repeating. But what if I schedule 2 incremental
>>>> tasks?
>>>>
>>>>
>>>> On Wednesday, January 14, 2015 at 3:07:47 PM UTC+1, Richard Wallis
>>>> wrote:
>>>>>
>>>>> Just an update, there's a bug in my original implementation which
>>>>> causes the loopCounter to tend to increase until the task ends.
>>>>>
>>>>> After removing the bug, the performance increase is only about 8% for
>>>>> the fibonacci sequence.
>>>>>
>>>>> Also the safer version where the loopCounter increments by at most 1
>>>>> eliminates the performance gain for the fibonacci test because the task
>>>>> runs 8000 to 9000 times during each loop so the loopCounter never gets
>>>>> anywhere close to a number that will make a difference.
>>>>>
>>>>>
>>>>> On Wed, Jan 14, 2015 at 2:19 PM, Richard Wallis <rdwal...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Actually in defense of adding the optimization to the current api.
>>>>>>
>>>>>> It can be made safe by only ever setting lastLoopCounter to
>>>>>> min(lastLoopCounter + 1, loopCounter).
>>>>>>
>>>>>> That should cause similar improved performance for multiple second
>>>>>> tasks without ever having a lower frame rate than the current scheduler.
>>>>>>
>>>>>> On Wed, Jan 14, 2015 at 1:41 PM, Richard Wallis <rdwal...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> You can't use webworkers to process the dom / xml, or do layout
>>>>>>> which is pretty much all my long running tasks do.
>>>>>>>
>>>>>>> Agree this is too dangerous to just add to the existing api but if
>>>>>>> you added a new method scheduleConstantTime() that ran the optimization
>>>>>>> it
>>>>>>> might be useful.
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Jan 14, 2015 at 1:27 PM, 'Daniel Kurka' via GWT Contributors
>>>>>>> <google-web-toolkit-contributors@googlegroups.com> wrote:
>>>>>>>
>>>>>>>> I think we do not want to make any assumptions about runtime of any
>>>>>>>> tasks since this would not work well with task that have variable
>>>>>>>> runtime.
>>>>>>>>
>>>>>>>> If you need to do heavy calculations since browser APIs have
>>>>>>>> evolved a lot you should be using web workers anyway.
>>>>>>>>
>>>>>>>> On Wed, Jan 14, 2015 at 11:57 AM, Richard Wallis <
>>>>>>>> rdwal...@gmail.com> wrote:
>>>>>>>>
>>>>>>>>> At the moment, incremental scheduler runs a repeating task in a
>>>>>>>>> while loop that checks if its duration has exceeded 16ms and then
>>>>>>>>> returns
>>>>>>>>> control to the browser.
>>>>>>>>>
>>>>>>>>> If you assume that an incrementally scheduled task will tend to
>>>>>>>>> run in about the same time as it did before then you can get about a
>>>>>>>>> 10%
>>>>>>>>> speed up by counting the number of times the task ran during the last
>>>>>>>>> duration loop and then running the task that many times at the start
>>>>>>>>> (and
>>>>>>>>> skipping the duration check).
>>>>>>>>>
>>>>>>>>> On chrome a task that calculated the fibonacci sequence managed to
>>>>>>>>> calculate about 450000 numbers in a second with the current scheduler
>>>>>>>>> and
>>>>>>>>> about 500000 numbers in the same time with my new one below.
>>>>>>>>>
>>>>>>>>> And a realworld task that parses xml dropped from taking about 50
>>>>>>>>> seconds each run to 42 seconds.
>>>>>>>>>
>>>>>>>>> Of course if the incrementally scheduled task tends to take a
>>>>>>>>> longer and longer time to complete on each run the oprimization might
>>>>>>>>> cause
>>>>>>>>> issues so maybe we need to create a new kind of task scheduler
>>>>>>>>> specifically
>>>>>>>>> for tasks that tend to complete their runs in a similar timeframe.
>>>>>>>>>
>>>>>>>>> I'm looking for critiscim of the code below, (and maybe someone to
>>>>>>>>> take ownership of this and do a PR to gerrit on my behalf)
>>>>>>>>>
>>>>>>>>> In SchedulerImpl replace the current runRepeatingTasks method with
>>>>>>>>> this: (lastLoopCount is a private int field);
>>>>>>>>>
>>>>>>>>> /**
>>>>>>>>> * Execute a list of Tasks that hold RepeatingCommands.
>>>>>>>>> *
>>>>>>>>> * @return A replacement array that is possibly a shorter copy
>>>>>>>>> of <code>tasks</code>
>>>>>>>>> */
>>>>>>>>> private JsArray<Task> runRepeatingTasks(JsArray<Task> tasks) {
>>>>>>>>> assert tasks != null : "tasks";
>>>>>>>>>
>>>>>>>>> int length = tasks.length();
>>>>>>>>> if (length == 0) {
>>>>>>>>> return null;
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> boolean canceledSomeTasks = false;
>>>>>>>>>
>>>>>>>>> Duration duration = createDuration();
>>>>>>>>> int loopCount = 0;
>>>>>>>>> outer: while (duration.elapsedMillis() < TIME_SLICE) {
>>>>>>>>>
>>>>>>>>> if (length == 1) {
>>>>>>>>> while (lastLoopCount-- > 0 ) {
>>>>>>>>> if (!tasks.get(0).executeRepeating()) {
>>>>>>>>> tasks.set(0, null);
>>>>>>>>> canceledSomeTasks = true;
>>>>>>>>> break outer;
>>>>>>>>> }
>>>>>>>>> loopCount += 1;
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>>> boolean executedSomeTask = false;
>>>>>>>>> for (int i = 0; i < length; i++) {
>>>>>>>>> assert tasks.length() == length : "Working array length
>>>>>>>>> changed " + tasks.length() + " != "
>>>>>>>>> + length;
>>>>>>>>> Task t = tasks.get(i);
>>>>>>>>> if (t == null) {
>>>>>>>>> continue;
>>>>>>>>> }
>>>>>>>>> executedSomeTask = true;
>>>>>>>>>
>>>>>>>>> assert t.isRepeating() : "Found a non-repeating Task";
>>>>>>>>>
>>>>>>>>> if (!t.executeRepeating()) {
>>>>>>>>> tasks.set(i, null);
>>>>>>>>> canceledSomeTasks = true;
>>>>>>>>> }
>>>>>>>>> loopCount += 1;
>>>>>>>>> }
>>>>>>>>> if (!executedSomeTask) {
>>>>>>>>> // no work left to do, break to avoid busy waiting until
>>>>>>>>> TIME_SLICE is reached
>>>>>>>>> break;
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>>> if (length == 1) {
>>>>>>>>> lastLoopCount = loopCount;
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> if (canceledSomeTasks) {
>>>>>>>>> lastLoopCount = 0;
>>>>>>>>> JsArray<Task> newTasks = createQueue();
>>>>>>>>> // Remove tombstones
>>>>>>>>> for (int i = 0; i < length; i++) {
>>>>>>>>> if (tasks.get(i) != null) {
>>>>>>>>> newTasks.push(tasks.get(i));
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>>> assert newTasks.length() < length;
>>>>>>>>> return newTasks.length() == 0 ? null : newTasks;
>>>>>>>>> } else {
>>>>>>>>> return tasks;
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> You received this message because you are subscribed to the Google
>>>>>>>>> Groups "GWT Contributors" group.
>>>>>>>>> To unsubscribe from this group and stop receiving emails from it,
>>>>>>>>> send an email to google-web-toolkit-contributors+unsubscribe@
>>>>>>>>> googlegroups.com.
>>>>>>>>> To view this discussion on the web visit
>>>>>>>>> https://groups.google.com/d/msgid/google-web-toolkit-
>>>>>>>>> contributors/80297aa7-c49a-4e3b-b70a-554bfaef52f0%
>>>>>>>>> 40googlegroups.com
>>>>>>>>> <https://groups.google.com/d/msgid/google-web-toolkit-contributors/80297aa7-c49a-4e3b-b70a-554bfaef52f0%40googlegroups.com?utm_medium=email&utm_source=footer>
>>>>>>>>> .
>>>>>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Google Germany GmbH
>>>>>>>> *Dienerstr. 12*
>>>>>>>> *80331 München*
>>>>>>>>
>>>>>>>> Registergericht und -nummer: Hamburg, HRB 86891
>>>>>>>> Sitz der Gesellschaft: Hamburg
>>>>>>>> Geschäftsführer: Graham Law, Katherine Stephens
>>>>>>>>
>>>>>>>> --
>>>>>>>> You received this message because you are subscribed to the Google
>>>>>>>> Groups "GWT Contributors" group.
>>>>>>>> To unsubscribe from this group and stop receiving emails from it,
>>>>>>>> send an email to google-web-toolkit-contributors+unsubscribe@
>>>>>>>> googlegroups.com.
>>>>>>>> To view this discussion on the web visit
>>>>>>>> https://groups.google.com/d/msgid/google-web-toolkit-contributors/
>>>>>>>> CALLujipuOFMxCnLjdM_FER4n3-eAHPEWUQGf3AZENR_ezq255A%
>>>>>>>> 40mail.gmail.com
>>>>>>>> <https://groups.google.com/d/msgid/google-web-toolkit-contributors/CALLujipuOFMxCnLjdM_FER4n3-eAHPEWUQGf3AZENR_ezq255A%40mail.gmail.com?utm_medium=email&utm_source=footer>
>>>>>>>> .
>>>>>>>>
>>>>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups "GWT Contributors" group.
>>>> To unsubscribe from this group and stop receiving emails from it, send
>>>> an email to
>>>> google-web-toolkit-contributors+unsubscr...@googlegroups.com.
>>>> To view this discussion on the web visit
>>>> https://groups.google.com/d/msgid/google-web-toolkit-contributors/26df3965-32f3-482d-a3e5-a7bba432d427%40googlegroups.com
>>>> <https://groups.google.com/d/msgid/google-web-toolkit-contributors/26df3965-32f3-482d-a3e5-a7bba432d427%40googlegroups.com?utm_medium=email&utm_source=footer>
>>>> .
>>>>
>>>> For more options, visit https://groups.google.com/d/optout.
>>>>
>>>
>>>
>>
>
--
You received this message because you are subscribed to the Google Groups "GWT
Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/google-web-toolkit-contributors/CAEqaEVhfD2735UxzZ_qqYm-7d5EmD%2BfibiogC5SOA4PuzNiXgw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
