remove boost from GUIFontCache #8996

FernetMenta · 2016-01-30T23:18:47Z

see title
superseds #7495

xbmc/guilib/GUIFontCache.cpp


  EntryList m_list;
  CGUIFontCache<Position, Value> *m_parent;
+  std::map<size_t, Value*> test;


FernetMenta · 2016-01-31T08:56:06Z

updated. this is basically the same behaviour without the boost clutter
@popcornmix you reported issues on #7495 could you test this on Pi please?

Paxxi · 2016-01-31T09:52:53Z

Will this have the same performance chracteristics as the multi_index solution? Not sure that it matters in real life use though. I'll close mine so we can focus work here.

FernetMenta · 2016-01-31T10:14:58Z

I can't think of any reason why it should perform worse. Maybe it is even better because index of age is simplified. Writing this I think I access this index from the wrong end :)

xbmc/guilib/GUIFontCache.cpp

-  typedef typename EntryList::template index<Hash>::type::iterator EntryHashIterator;
+  struct EntryList
+  {
+    typedef std::multimap<size_t, CGUIFontCacheEntry<Position, Value>*> HashMap;


Paxxi · 2016-01-31T11:01:56Z

hehe :) No real objections to the solution from my side but there's a few minors.

method names should follow naming standard with uppercase first letter
use std::make_pair instead of pair constructor(could make it a bit more readable)

Interested to see the testing by @popcornmix and @MilhouseVH how it performs.

FernetMenta · 2016-01-31T11:19:16Z

use std::make_pair instead of pair constructor(could make it a bit more readable)

I used value_type as suggested by the docs. This make the code shorter because you don't need the template stuff

FernetMenta · 2016-01-31T11:28:57Z

lets see how it compiles on the various platforms
jenkins build this please

popcornmix · 2016-01-31T12:10:24Z

This causes a performance regression.
On Pi2 with RSS enabled on confluence main menu with VIDEOS highlighted, I initially get ~47fps with both the original and with this PR.
However as time elapses the fps goes down with this PR. After 30 minutes I'm at 25fps. This slow down doesn't occur with original code.
I suspect cache entries are not being released correctly making cache lookups slower with time.

xbmc/guilib/GUIFontCache.cpp

 {
-  delete &m_key.m_colors;
-  delete &m_key.m_text;


FernetMenta · 2016-01-31T12:26:20Z

@popcornmix can you log the size of HashMap every couple of seconds?

afedchin · 2016-01-31T12:54:32Z

@FernetMenta HashMap size isn't changed, but agemap is decreased gradually every few minutes

FernetMenta · 2016-01-31T13:07:21Z

@popcornmix @afedchin you were right. there was an issue with AgeMap.erase was called with wrong iterator

Paxxi · 2016-01-31T13:15:09Z

I tested my theory and it works fine on gcc which is picky about these things.
Paxxi/xbmc@ba3862b

popcornmix · 2016-01-31T13:25:21Z

With "squash me" commit it looks okay. Still ~47fps after 15 minutes.
Might be worth waiting for feedback from tonight's Milhouse build as this code is pretty critical for performance.

afedchin · 2016-01-31T13:25:22Z

@FernetMenta seems now it works as expected. both sizes of maps are identical and constant

FernetMenta · 2016-01-31T15:04:47Z

jenkins build this please

Paxxi · 2016-01-31T15:50:27Z

@FernetMenta reverted the make_pair changes Paxxi/xbmc@d76d467

FernetMenta · 2016-02-01T06:49:54Z

jenkins build this please

FernetMenta · 2016-02-01T10:10:44Z

@popcornmix @MilhouseVH did you hear any issues related to this?

MilhouseVH · 2016-02-01T10:16:48Z

None reported so far, but it's only been 12 hours (though 82 installations).

popcornmix · 2016-02-01T10:21:12Z

I'm okay with this being merged. We'll let you know if any issues show up down the line, but I think this is probably fine.

remove boost from GUIFontCache

MilhouseVH · 2016-02-06T15:16:47Z

I'm seeing evidence of text/font "glitching" which is a result of this PR, but only with OpenELEC RPi/RPi2 builds and not OpenELEC x86 builds.

I also do not see the glitching with Kodi on Ubuntu x86, so it may be GL/GLES related.

In my RPi test builds the problem first appears in build #0131, which is when I introduced this PR.

In a test build I have reverted this PR on RPi/RPi2 (and also #9021, restoring Boost) and the glitching problem went away, so it's definitely a result of this PR.

The problem only seems to affect the debug overlay text, I haven't seen issues with other non-debug GUI text, but it may be a prelude to further problems down the line.

See the following frame grabs from this video (Dropbox, 62MB):

Shadow of middle line overlays top line:
Shadow of bottom line is much lower than it should be:
Foreground text (without shadow) from middle line overlays itself:

To reproduce:

Configure Dim screensaver, 20% opacity (not sure opacity matters, but it's what I'm using), timeout doesn't matter. The Black screensaver can also be used to reproduce this issue
Enable debugging with debug overlay
Click "Preview" (or wait) for Dim/Black screensaver to activate

Result:

Debug overlay text starts to visibly flicker and glitch as foreground and shadow text is displayed at the wrong coordinates

Glitching of the debug overlay text continues after the screensaver is deactivated.

Using a screensaver such as Shader Toy does NOT cause this problem. I haven't tested any other screensavers.

Edit: Fixed by #9065

FernetMenta added Type: Cleanup non-breaking change which removes non-working or unmaintained functionality v17 Krypton labels Jan 30, 2016

afedchin reviewed Jan 31, 2016
View reviewed changes

xbmc/guilib/GUIFontCache.cpp

EntryList m_list;

CGUIFontCache<Position, Value> *m_parent;

std::map<size_t, Value*> test;

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

FernetMenta force-pushed the guifont branch from 2c286c6 to 9fd269d Compare January 31, 2016 08:51

FernetMenta mentioned this pull request Jan 31, 2016

[RFC][FONTCACHE] get rid of boost multi_index #7495

Closed

Paxxi reviewed Jan 31, 2016
View reviewed changes

FernetMenta force-pushed the guifont branch from b483b1e to 77ff892 Compare January 31, 2016 11:27

Paxxi reviewed Jan 31, 2016
View reviewed changes

xbmc/guilib/GUIFontCache.cpp

{

delete &m_key.m_colors;

delete &m_key.m_text;

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

FernetMenta force-pushed the guifont branch from 9fe9a5d to b2b8d19 Compare January 31, 2016 15:04

remove boost from GUIFontCache

8f1ffc9

FernetMenta force-pushed the guifont branch from 7b3f232 to 8f1ffc9 Compare February 1, 2016 06:49

FernetMenta added a commit that referenced this pull request Feb 1, 2016

Merge pull request #8996 from FernetMenta/guifont

9c32dff

remove boost from GUIFontCache

FernetMenta merged commit 9c32dff into xbmc:master Feb 1, 2016

FernetMenta deleted the guifont branch February 1, 2016 10:21

MartijnKaijser modified the milestone: Krypton 17.0-alpha1 Feb 1, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove boost from GUIFontCache #8996

remove boost from GUIFontCache #8996

FernetMenta commented Jan 30, 2016

This comment was marked as spam.

This comment was marked as spam.

FernetMenta commented Jan 31, 2016

Paxxi commented Jan 31, 2016

FernetMenta commented Jan 31, 2016

This comment was marked as spam.

This comment was marked as spam.

Paxxi commented Jan 31, 2016

FernetMenta commented Jan 31, 2016

FernetMenta commented Jan 31, 2016

popcornmix commented Jan 31, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

FernetMenta commented Jan 31, 2016

afedchin commented Jan 31, 2016

FernetMenta commented Jan 31, 2016

Paxxi commented Jan 31, 2016

popcornmix commented Jan 31, 2016

afedchin commented Jan 31, 2016

FernetMenta commented Jan 31, 2016

Paxxi commented Jan 31, 2016

FernetMenta commented Feb 1, 2016

FernetMenta commented Feb 1, 2016

MilhouseVH commented Feb 1, 2016

popcornmix commented Feb 1, 2016

MilhouseVH commented Feb 6, 2016

remove boost from GUIFontCache #8996

remove boost from GUIFontCache #8996

Conversation

FernetMenta commented Jan 30, 2016

This comment was marked as spam.

This comment was marked as spam.

FernetMenta commented Jan 31, 2016

Paxxi commented Jan 31, 2016

FernetMenta commented Jan 31, 2016

This comment was marked as spam.

This comment was marked as spam.

Paxxi commented Jan 31, 2016

FernetMenta commented Jan 31, 2016

FernetMenta commented Jan 31, 2016

popcornmix commented Jan 31, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

FernetMenta commented Jan 31, 2016

afedchin commented Jan 31, 2016

FernetMenta commented Jan 31, 2016

Paxxi commented Jan 31, 2016

popcornmix commented Jan 31, 2016

afedchin commented Jan 31, 2016

FernetMenta commented Jan 31, 2016

Paxxi commented Jan 31, 2016

FernetMenta commented Feb 1, 2016

FernetMenta commented Feb 1, 2016

MilhouseVH commented Feb 1, 2016

popcornmix commented Feb 1, 2016

MilhouseVH commented Feb 6, 2016