Opened 10 years ago

Closed 10 years ago

#7616 closed patch (fixed)

Revert settings cache behaviour introduced in [20828]

Reported by: danielk Owned by: danielk
Priority: minor Milestone: 0.23
Component: MythTV - General Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

We stopped caching default values, which caused a noticeable performance regression in playback startup. I'm attaching two patches to this ticket, one for trunk, the other for 0.22-fixes.

I asked Michael T. Dean about this and the main problem this was trying to address was the caching of defaults when Get{,Num,Float}Setting was called without a default. Then "", 0, and 0.0 would get cached. To address this problem without the performance implications of not caching defaults at all we use a sentinel value of "<settings_sentinel_value>" and never cache that value. The secondary problem this tried to solve was Get{,Num,Float}Settings called with different defaults in two or more places, this coding error should not cause the types of problems that caching "", 0, and 0.0 can cause since anyone providing a default should be picking a sane one.

Note: There is a bit of unneeded redundancy in the settings code, I will be addressing that in trunk in separate patch.

Attachments (2)

7616-trunk-v1.patch (6.0 KB) - added by danielk 10 years ago.
untested initial patch
7616-fixes-v1.patch (6.4 KB) - added by danielk 10 years ago.
untested patch

Download all attachments as: .zip

Change History (4)

Changed 10 years ago by danielk

Attachment: 7616-trunk-v1.patch added

untested initial patch

Changed 10 years ago by danielk

Attachment: 7616-fixes-v1.patch added

untested patch

comment:1 Changed 10 years ago by danielk

(In [22917]) Refs #7616. Revert to original setting cache behaviour with respect to default values.

This was changed a couple times to not cache defaults, causing some massive slowdowns in the app. This reverts to caching default values when spevifies, but avoids doing so when no default value is not explicitly specified.

comment:2 Changed 10 years ago by danielk

Resolution: fixed
Status: newclosed

Since the next release is fairly soon I'm not going to try to backport this to 0.22-fixes

Note: See TracTickets for help on using tickets.