Re: WIP: new VCL scheduler feature branch

Hi Noel,

thanks for the feedback.

Am 17.09.2016 um 13:54 schrieb Noel Grandin:
> Nice work!
>
> Some random low priority comments:
>
> In "Just walk the task list once per timeout"
> https://cgit.freedesktop.org/libreoffice/core/commit/?h=feature/new-vcl-scheduler&id=435e21fe0330436e76e5e053d5d5d94df734a554
> The evaluate_entry label doesn't make the code any easier to read.

So you're suggestion is? Introduce an other level of indention for the
if? The while loop is short enough and the goto labels are just used in
there. If you think it improves readability, I can change it.

> In "Reorganize Scheduler priority classes"
> https://cgit.freedesktop.org/libreoffice/core/commit/?h=feature/new-vcl-scheduler&id=5e4361e84607fc6d7623b31630505da7c934b945
>  (1) you haven't used a consistent mapping from old to new - sometimes
> LOW maps to HIGH_IDLE, sometimes HIGHEST maps to HIGH_IDLE. Perhaps
> there is a reason for this?

From naming and code reading I tried to guess, if the idle should be
processed before drawing. As I wrote I would like to get a comment or
annotation for all non-default priorities, i.e. all calls to "SetPriority".

I have no idea, if my guesses are correct, just that mail merge is much
faster now and LO is still usable while the mail merge runs. Actually
you can see the slowdown of mail merge when doing stuff in LO :-)

> (2) I would call HIGH_IDLE, either PRE_RESIZE or BEFORE_RESIZE, because
> it's not any kind of IDLE anymore. I would call DEFAULT_IDLE just IDLE.

I just took the priority names from glib. I would keep the DEFAULT_IDLE,
or rename DEFAULT to TIMER? OTOH the Scheduler class sets the default
priority...

> In "Handle all main loop and task events"
> https://cgit.freedesktop.org/libreoffice/core/commit/?h=feature/new-vcl-scheduler&id=102c41c2e429bee489334361536779aa298bc181
>     -    bProcessedEvent = bProcessedEvent || bScheduledEevent;
>     +    bProcessedEvent |= bScheduledEevent;

Will do.

> In "Run Idle tasks immediatly"
> https://cgit.freedesktop.org/libreoffice/core/commit/?h=feature/new-vcl-scheduler&id=a1ecb280872c5487615558f8d140a380ef3e0d36
> It occurs to me that when we get an overload condition, it would be very
> helpful to dump the names of the current events.

Dumping the current event would probably not help much. But we could
dump the whole scheduler event state. OTOH you can dump it in GDB with

p *ImplGetSVData()->mpFirstSchedulerData

BTW: there are some LOK-Tests, which generate more then 1000 events. The
1000 is arbitrary from my POV. Probably checking the runtime of
ProcessAllPendingEvents() would be better?

Thanks,

Jan-Marek