perf: de-prioritize rendering non-visible tiles

[sgothel]

• Resolves: #9989
• Target version: master

Summary

Only render invisible combined-tiles if IDLE

As an attempt to increase responsiveness for client, we only send invisible requested combined-tiles if IDLE. IDLE is determined if socket poll timed out, i.e. nothing else to do for client.

Checklist

• [ ] I have run make prettier-write and formatted the code.
• [ ] All commits have Change-Id
• [ ] I have run tests with make check
• [ ] I have issued make run and manually verified that everything looks okay
• [ ] Documentation (manuals or wiki) has been updated or is not required

[sgothel]

The TileCombined's dimension, i.e. its _width and _height, is not fixed in the streaming TileCombined ctor (line ~404). Only the 1st TileDesc dimenstion is being stored. The combined dimension seems to be used at doRender only? If fixed, i.e. AABBox style grown (-> Rectangle) we simply could use it to determine visibility .. instead of looping through all recreated TileDesc instances.

[sgothel]

I need clarification regarding the IDLE criteria, which shall allow sending the invisible tiles. (timeout = to) This patch uses the to_left < time_consumed in drainedQueue, which works if initial to is reasonable reflecting time left. Testing showed that a to > 0 is only given for IDLE periods?

[sgothel]

Force push resolved merge conflict with latest master tip as detected with the bots. I must have pushed it this morning in-between.

[mmeeks]

It would be good to cleanup the ordering functionality at the same time though eg.

// FIXME: it's not that clear what good this does for us ... // we process all previews in the same batch of rendering void KitQueue::deprioritizePreviews()

Around thumbnailing slides - while ensuring that whatever algorithm we use doesn't create starvation problems.

[sgothel]

It would be good to cleanup the ordering functionality at the same time though eg.

// FIXME: it's not that clear what good this does for us ... // we process all previews in the same batch of rendering void KitQueue::deprioritizePreviews()

Looking at it, it only pushes leading preview-tiles to the end of the queue and you mentioned in the comment that a tile-req batch either only consist out of such preview-tiles or none shall be included(?).

Our IDLE/invisible mechanism act above the produced 'popWholeTileQueue' on the produced combined tiles, i.e. included this preview reordering. Not sure if this is the discussion here.

Around thumbnailing slides - while ensuring that whatever algorithm we use doesn't create starvation problems.

Since out IDLE/invisible mechanism includes the preview-tile order, see above, this should be well included in the solution.

[sgothel]

Offered the optional Tile/TileCombined intersection test patch via https://github.com/CollaboraOnline/online/pull/10197

[sgothel]

COOL 24.04 Cypress (desktop) — FAILURE: Passed locally: make check-desktop spec=writer/invalidations_spec.js

[sgothel]

rebased to master a330f9e75717de96c72794d0ac672f75376c9711 (no changes)

[sgothel]

re CodeQL see https://github.com/CollaboraOnline/online/pull/9916#issuecomment-2415664161

[sgothel]

PR #10183 as well as PR #10512 were rebased (forced pushed) twice

• first rebase
  • fixed loIdle determination
  • Document::drainQueue returns DocDrainStats for logging details, also reducing code duplication in KitSocketPoll::kitPoll
• second rebase
  • rebase to master

[sgothel]

I guess the obvious question then is if #10512 just "tidies things up" or if this pr on its own isn't sufficient and the other is needed to work properly?

Yes. The idle behavior in discussion may even render idle determination useless. In such case we wouldn't care and just sent the invisible tiles at last and one tile per sliced-loop from KitPoll. Otherwise, having the idleHint helps to exclude certain idle-determinations when LO is busy, i.e. claiming high-priority.

[sgothel]

Updated based on timeout (remaining time) only, disregarding potential open-ended idle/wakeup case (note)

Due to ambiguous timeout==0 semantics w/o priority (idle or intentional zero timeout), we are not able to determine general idleness w/o passing this information down to COOL. (Post reviewing LO's Task, Timer and Scheduler, confirmed by vcl/README.scheduler.md)

Updated description accordingly.

[mmeeks]

Abandon this for now.