Opened 18 years ago
Closed 18 years ago
Last modified 18 years ago
#1002 closed defect (fixed)
Playback Pessimization
Reported by: | Owned by: | danielk | |
---|---|---|---|
Priority: | minor | Milestone: | 0.19 |
Component: | mythtv | Version: | head |
Severity: | medium | Keywords: | playback |
Cc: | Ticket locked: | no |
Description
The attached patch removes a serious pessimization from MythTV's playback code.
The frontend was doing a terribly slow regexp creation/search sequence in TV::GetQueuedChanNum? that was soaking 14.6% of the process' CPU cycles.
Attachments (1)
Change History (6)
Changed 18 years ago by
Attachment: | tv_play.patch added |
---|
comment:1 Changed 18 years ago by
Milestone: | unknown → 0.19 |
---|---|
Owner: | changed from Isaac Richards to danielk |
Priority: | major → minor |
Severity: | high → medium |
comment:2 Changed 18 years ago by
I believe the code is recompiling them every time, but regardless, it's the search that's soaking the cycles.
Assuming that dynamic regexps will be fast may be a MythTV problem.
I *seriously* doubt that Qt has a specially pessimized regex engine for the mac. They surely use one piece of code on all platforms. Have you profiled this on any other platforms? Do you have any reason to think it isn't sucking as many cycles on Linux?
comment:3 Changed 18 years ago by
I've profiled a half dozen times since this code went in and it never showed up in the top 100...
Perhaps the usleep(1000) in the tv_play loop waits longer in a typical Linux install than OSX?
In anycase, I'll optimize this bit of code, but you may wish to discover the underlying cause of the problem.
comment:4 Changed 18 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [8568]) Closes #1002, by optimizing GetQueuedChanNum?().
If there is nothing in queuedChanNum we just return it without running the regular expression matching at all.
We also use a static regular expression which means we don't rely on the Qt regular expression cache to avoid recompiling the matching tables, this Qt feature appears to be broken or missing in the MacOSX Qt implementation.
Finally, we use minimal rather than greedy matching so the expression can be evaluated more quickly.
comment:5 Changed 18 years ago by
- Uses an instance variable for the GetQueuedChanNum?() regular expression; to avoid some problems reported with QRegExp not being properly reenterant in Qt 3.2.
- Uses a QMutex to make sure access to the regular expression and the queued input strings are safe.
- Adds a few QString deep copies to avoid problems with the non-threadsafe QString reference counting since the input strings are updated by 3 threads.
Does the Mac Qt not compile regular expressions? Or is this particular code somehow breaking the compiled Qt regular expression caching?
There are many other places where we use regular expressions on the assumption that they will execute quickly, this could be a larger issue for the OSX port.