On 2016/09/19 2:27 PM, Jan-Marek Glogowski wrote:
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.
No, just inline that single line of code at the evaluate_entry label and then jump to the exit
label.
But that's just my taste, if you disagree, feel free to ignore.
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 :-)
OK, just checking, because from the commit message it looks like a mechanical change, but then the mapping was not
consistent.
(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...
Ah, then probably leave them alone, if they map to something we already use.
Context
Privacy Policy |
Impressum (Legal Info) |
Copyright information: Unless otherwise specified, all text and images
on this website are licensed under the
Creative Commons Attribution-Share Alike 3.0 License.
This does not include the source code of LibreOffice, which is
licensed under the Mozilla Public License (
MPLv2).
"LibreOffice" and "The Document Foundation" are
registered trademarks of their corresponding registered owners or are
in actual use as trademarks in one or more countries. Their respective
logos and icons are also subject to international copyright laws. Use
thereof is explained in our
trademark policy.