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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> >> 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 <[email protected]> >>> 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 <[email protected]> >>>>> 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 <[email protected]> >>>>>> 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 >>>>>>> <[email protected]> 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 < >>>>>>>> [email protected]> 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 >>>> [email protected]. >>>> 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 [email protected]. 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.
