Modify

Opened 7 years ago

Last modified 3 weeks ago

#8962 assigned Developer Task

playbackbox.cpp:extract_job_state() inefficient

Reported by: danielk Owned by: wagnerrp
Priority: minor Milestone: 29.2
Component: MythTV - General Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

Right now it queries the info independently for every recording on screen. But it is much more efficient to get this info in MainServer::HandleQueryRecordings?() where we already do a partial bulk load of this info to set FL_COMMPROCESSING.

Attachments (4)

8962_v1.patch (5.3 KB) - added by stichnot 3 years ago.
8962_v2.patch (8.0 KB) - added by stichnot 3 years ago.
8962_0.27_v1.patch (10.0 KB) - added by stichnot 3 years ago.
Untested backport to fixes/0.27
8962_0.27_v2.patch (8.1 KB) - added by stichnot 3 years ago.
Add fixes from master

Download all attachments as: .zip

Change History (19)

comment:1 Changed 7 years ago by robertm

  • Status changed from new to assigned

comment:2 Changed 7 years ago by stuartm

  • Milestone 0.25 deleted

Milestone 0.25 deleted

comment:3 Changed 7 years ago by stuartm

  • Owner changed from danielk to stuartm
  • Status changed from assigned to accepted
  • Type changed from task to Developer Task

comment:4 Changed 6 years ago by stuartm

  • Milestone changed from 0.25 to 0.26

comment:5 Changed 5 years ago by kenni

  • Milestone changed from 0.26 to 0.27

comment:6 Changed 5 years ago by wagnerrp

  • Owner changed from stuartm to wagnerrp
  • Status changed from accepted to assigned

I'm picking this up as part of the rework for #7990.

comment:7 Changed 5 years ago by wagnerrp

  • Milestone changed from 0.27 to 0.28

comment:8 Changed 3 years ago by stichnot

I agree that in principle it would be nicer to add this directly to ProgramInfo? and ProgramList? and benefit from automatic PBB updates when changes are made.

However, it is complicated by the fact that a given ProgramInfo? can have multiple job queue records associated with it - commflag, transcode, metadata lookup, user jobs - and each can be in a different state, so it's hard to pack that all into a single ProgramInfo? field. Unless we're committed to only transferring a summary of specific state information about commflag and transcode jobs only, and encode that into a single field.

In the meantime, I've attached a simple patch that locally caches the entire job queue and reloads it at most once every 5 seconds. This greatly improves the sluggishness of navigating the Watch Recordings screen.

By the way, at least with my theme, every cursor move in the Watch Recordings screen fires off 4 jobqueue queries for each item displayed (plus a few extra queries). With this patch, changing the selection creates 3 queries total - 2 for card inputname and one for bookmark position (for preview generation) - plus 1 jobqueue query if it's been 5 seconds since the last query. Not perfect, but an order of magnitude better than before.

Changed 3 years ago by stichnot

Changed 3 years ago by stichnot

comment:9 Changed 3 years ago by Jim Stichnoth <jstichnoth@…>

In b43b11ca3062086d20ee0072368e9dd3646a5faa/mythtv:

Reduce "Watch Recordings" sluggishness by caching jobqueue state.

Refs #8962, refs #7990.

Currently, when navigating the Watch Recordings screen, every time the
cursor position is moved, 4 jobqueue queries are issued for each item
visible on the screen (the actual number being controlled by the
theme). These queries are done within the UI thread, which makes
navigation noticeably sluggish on most any setup that isn't a
moderately powerful combined frontend/backend.

Ideally, any time a job's status changes (including queue addition and
removal), we would call ProgramInfo::SendUpdateEvent?() to get the
Watch Recordings entry updated, along with any other
ProgramInfoUpdater? subscribers. Also,
ProgramInfo::LoadProgramFromRecorded?() and LoadFromRecorded?() would
set up the necessary commflag and transcode flags in the ProgramInfo?
object.

However, in the current jobqueue implementation, this is not very
practical because of functions like:

JobQueue::DeleteJob?(int jobID)
JobQueue::ChangeJobCmds?(int jobID, int newCmds)
JobQueue::ChangeJobFlags?(int jobID, int newFlags)
JobQueue::ChangeJobStatus?(int jobID, int newStatus, QString comment)
JobQueue::ChangeJobComment?(int jobID, QString comment)
JobQueue::ChangeJobArgs?(int jobID, QString args)
JobQueue::ChangeJobHost?(int jobID, QString newHostname)
JobQueue::CleanupOldJobsInQueue?()

where the ProgramInfo? isn't readily available (though it could be
found given another query on jobID for all but the last function).

Instead, in this approach, the PlaybackBox? locally caches the jobqueue
contents, reloading every 15 seconds as needed. The reload blocks the
UI thread, but only for a small fraction of the time that the current
implementation was blocking it, so it's unlikely a user pay much
attention to this once-in-15-seconds query.

This code should be removed once #7990 makes jobs more autonomous.

Changed 3 years ago by stichnot

Untested backport to fixes/0.27

Changed 3 years ago by stichnot

Add fixes from master

comment:10 Changed 3 years ago by Jim Stichnoth <jstichnoth@…>

In 974d1ae9b6fe2f12b8cfc48aafcf10fc730e414e/mythtv:

Reduce "Watch Recordings" sluggishness by caching jobqueue state.

Refs #8962, refs #7990.

This is essentially a cherry-pick of b43b11ca.

comment:11 Changed 2 years ago by stuarta

  • Milestone changed from 0.28 to 0.29

comment:12 Changed 23 months ago by stuarta

  • Milestone changed from 0.29 to 29.0

Milestone renamed

comment:13 Changed 3 months ago by stuarta

  • Milestone changed from 29.0 to 29.1

comment:14 Changed 3 weeks ago by stuarta

  • Milestone changed from 29.1 to 0.28.2

Moving remaining open tickets to 0.28.2 milestone

comment:15 Changed 3 weeks ago by stuarta

  • Milestone changed from 0.28.2 to 29.2

Moving remaining open tickets to 29.2 milestone

Add Comment

Modify Ticket

Action
as assigned The owner will remain wagnerrp.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.