summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLuboš Luňák <l.lunak@collabora.com>2021-02-07 16:34:38 +0100
committerLuboš Luňák <l.lunak@collabora.com>2021-02-07 18:57:30 +0100
commitd3b498cc4732f964919fecb265085cefcc422469 (patch)
treeb5cc5bbc381e18a2cad621b75cbe7401db0c585c
parentf343ad6d8a78798814c9d38261043619b4e2b487 (diff)
invoke idle priority timers only when actually idle
The 'Idle' timers are misnamed. They are zero-timeout times, i.e. they are invoked immediately after returning to the main loop. But that does not necessarily mean they are invoked when idle, there may be e.g. user input pending in the system event queue. In fact, LO events are processed before system events, which means that 'Idle' timers are normally processed before user input. Besides being confused, this also leads to poor performance in some cases, such as when using mouse wheel to zoom in a large document. This results in several mouse wheel events, each of which will result in adjusting the zoom and that causing a repaint. Repaints are internally handled using a TaskPriority::REPAINT 'Idle', and so what happens is zoom->repaint->zoom->repaint->zoom->repaint instead of the more efficient zoom->zoom->zoom->repaint. This change (besides trying to clarify the confusion in the docs) delays invoking tasks with priorities TaskPriority::HIGH_IDLE and lower if there is user input or repaint events in the OS queue. That means that tasks using idle priorities actually will be invoked only when idle (barring background threads etc.). I'm reasonably certain this is a safe change, there's no guarantee when exactly tasks will be invoked (e.g. other tasks with a higher priority go first) and explicitly specifying such a priority means asking for it. I already implemented this once in 06d731428ef6cf93c7333e8228b, and it was also again done in 87199d3829257420429057336283, but apparently these have been removed. There was d348035a60361a1b9ba9e 'Drop special idle handling' with the reasoning that 'Idles are just instant timers'. Which strictly technically speaking is true due to 'Idle' being a misnomer, but the point is that some idles should be actual idles and that's why they need to be handled specially. Change-Id: I36c2b02a80ae7e1476b731f878d9b28aa87975f4 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/110538 Tested-by: Jenkins Reviewed-by: Luboš Luňák <l.lunak@collabora.com>
-rw-r--r--include/vcl/idle.hxx4
-rw-r--r--include/vcl/task.hxx1
-rw-r--r--vcl/README.scheduler15
-rw-r--r--vcl/source/app/scheduler.cxx11
4 files changed, 31 insertions, 0 deletions
diff --git a/include/vcl/idle.hxx b/include/vcl/idle.hxx
index ebbeb160cf41..78fcaae817b9 100644
--- a/include/vcl/idle.hxx
+++ b/include/vcl/idle.hxx
@@ -26,6 +26,10 @@
* An idle is a timer to be scheduled immediately.
*
* It's - more or less - just a convenience class.
+ *
+ * Note: Despite the name, the timer is not necessarily invoked when idle.
+ * Use an idle priority such as TaskPriority::DEFAULT_IDLE to actually
+ * invoke the timer only when idle.
*/
class VCL_DLLPUBLIC Idle : public Timer
{
diff --git a/include/vcl/task.hxx b/include/vcl/task.hxx
index d8adae7eff0b..e95aa5bc460a 100644
--- a/include/vcl/task.hxx
+++ b/include/vcl/task.hxx
@@ -28,6 +28,7 @@ enum class TaskPriority
{
HIGHEST, ///< These events should run very fast!
DEFAULT, ///< Default priority used, e.g. the default timer priority
+ // Input from the OS event queue is processed before HIGH_IDLE tasks.
HIGH_IDLE, ///< Important idle events to be run before processing drawing events
RESIZE, ///< Resize runs before repaint, so we won't paint twice
REPAINT, ///< All repaint events should go in here
diff --git a/vcl/README.scheduler b/vcl/README.scheduler
index 263fe329bc3d..38e17d8cedef 100644
--- a/vcl/README.scheduler
+++ b/vcl/README.scheduler
@@ -74,6 +74,21 @@ non-scheduler code, so it was converted to a non-recursive
std::mutex.
+= Idle processing =
+
+Confusingly, there are 2 concepts that are called 'idle':
+
+* Instant (zero timeout) tasks, represented e.g. by the Idle class. This is
+a misnomer, as these tasks are processed after returning to the main loop.
+This is not necessarily when LO is idle, in fact such tasks may be invoked
+while there is input in the OS event queue pending.
+(TODO: This case should be fixed by renaming.)
+
+* Low priority tasks, represented by priorities TaskPriority::HIGH_IDLE and lower.
+In addition to being invoked only when there is no task with a higher priority,
+pending input in the OS event queue also takes precedence.
+
+
= Lifecycle / thread-safety of Scheduler-based objects =
A scheduler object it thread-safe in the way, that it can be associated to
diff --git a/vcl/source/app/scheduler.cxx b/vcl/source/app/scheduler.cxx
index b325f9238a7d..57b8176521f7 100644
--- a/vcl/source/app/scheduler.cxx
+++ b/vcl/source/app/scheduler.cxx
@@ -430,6 +430,17 @@ bool Scheduler::ProcessTaskScheduling()
<< " of " << nTasks << " tasks" );
UpdateSystemTimer( rSchedCtx, nMinPeriod, true, nTime );
+ // Delay invoking tasks with idle priorities as long as there are user input or repaint events
+ // in the OS event queue. This will often effectively compress such events and repaint only
+ // once at the end, improving performance in cases such as repeated zooming with a complex document.
+ if ( pMostUrgent && pMostUrgent->mePriority >= TaskPriority::HIGH_IDLE
+ && Application::AnyInput( VclInputFlags::MOUSE | VclInputFlags::KEYBOARD | VclInputFlags::PAINT ))
+ {
+ SAL_INFO( "vcl.schedule", tools::Time::GetSystemTicks()
+ << " idle priority task " << pMostUrgent << " delayed, system events pending" );
+ pMostUrgent = nullptr;
+ }
+
if ( pMostUrgent )
{
SAL_INFO( "vcl.schedule", tools::Time::GetSystemTicks() << " "