#19191: Mac performance obtaining system fonts

[wxTrac]

#19191: Mac performance obtaining system fonts
-----------------------------------------+-------------------------
Reporter: andyr | Owner:
Type: optimization | Status: new
Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest
Keywords: Mac performance system font | Blocked By:
Blocking: | Patch: 1
-----------------------------------------+-------------------------
Big Sur, Intel.

I have an app which does frequent redrawing because it scrolls a waveform
across the window while playing the sound. On the Mac, Activity Monitor
said it was using 70% cpu so I did some very crude profiling - run it in
Xcode and pause it periodically, and see from the call stack what it is
doing. And I found that it was spending a surprising amount of time in
wxSystemSettingsNative::GetFont.

So I made the simple change that you will see in the patch, and my app
went down to 55% cpu.

Possibly this patch means that the app will not respond to changes the
user might make to the system default font? Not the end of the world from
my point of view.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19191>

[wxTrac]

#19191: Mac performance obtaining system fonts

---------------------------+-----------------------------------------

Reporter: andyr | Owner:
Type: optimization | Status: new
Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest

Resolution: | Keywords: Mac performance system font

Blocked By: | Blocking:
Patch: 1 |

---------------------------+-----------------------------------------
Changes (by andyr):

* Attachment "settings.patch" added.

[wxTrac]

#19191: Mac performance obtaining system fonts

---------------------------+-----------------------------------------

Reporter: andyr | Owner:
Type: optimization | Status: new
Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest

Resolution: | Keywords: Mac performance system font

Blocked By: | Blocking:
Patch: 1 |

---------------------------+-----------------------------------------

Comment (by vadz):

Thanks for the patch but while it's clearly an improvement from the
performance point of view, it probably breaks reacting to the system font
changes.

Under MSW we need to invalidate the cache when the system settings change,
can we do something similar under Mac? I.e. is there a notification that
we can subscribe to? Could we be already getting `wxSysColourChangedEvent`
when this happens?

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19191#comment:1>

[wxTrac]

#19191: Mac performance obtaining system fonts

---------------------------+-----------------------------------------

Reporter: andyr | Owner:
Type: optimization | Status: new
Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest

Resolution: | Keywords: Mac performance system font

Blocked By: | Blocking:
Patch: 1 |

---------------------------+-----------------------------------------

Comment (by vadz):

Stefan, do you think we should apply this (not in the current form, as we
should only create fonts when really needed, but we could still make them
static to cache them -- although we might need to have a `wxModule` to
destroy them properly, too) or should we just tell people that if they
worry about performance that much they should be caching the fonts
themselves?

OTOH wxGTK and wxMSW do already cache fonts like this, so arguably wxOSX
should do it too...

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19191#comment:2>

[wxTrac]

#19191: Mac performance obtaining system fonts

---------------------------+-----------------------------------------

Reporter: andyr | Owner:
Type: optimization | Status: new
Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest

Resolution: | Keywords: Mac performance system font

Blocked By: | Blocking:
Patch: 1 |

---------------------------+-----------------------------------------

Comment (by csomor):

> Under MSW we need to invalidate the cache when the system settings
change, can we do something similar under Mac? I.e. is there a
notification that we can subscribe to? Could we be already getting
`wxSysColourChangedEvent` when this happens?

I am not aware of an official way to change the system font under the
latest systems, nor of an event that would indicate such a change. There
used to be a setting in older systems and some tweaking tools allowed this
also on later systems. But this then is not something that we'd have to
support while running the app IMHO.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19191#comment:3>

[wxTrac]

#19191: Mac performance obtaining system fonts

---------------------------+-----------------------------------------

Reporter: andyr | Owner:
Type: optimization | Status: new
Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest

Resolution: | Keywords: Mac performance system font

Blocked By: | Blocking:
Patch: 1 |

---------------------------+-----------------------------------------

Comment (by csomor):

Replying to [comment:2 vadz]:

> Stefan, do you think we should apply this (not in the current form, as
we should only create fonts when really needed, but we could still make
them static to cache them -- although we might need to have a `wxModule`
to destroy them properly, too) or should we just tell people that if they
worry about performance that much they should be caching the fonts
themselves?
>
> OTOH wxGTK and wxMSW do already cache fonts like this, so arguably wxOSX
should do it too...

I'm already caching quite a lot for the usual fonts. So I think we should
do for these as well, we might do so in the {{{wxFont(wxOSXSystemFont
font)}}} constructor, have a hashmap of {{{wxFontRefData}}} and use
{{{wxOSXSystemFont}}} as key ?

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19191#comment:4>

[wxTrac]

#19191: Mac performance obtaining system fonts

---------------------------+-----------------------------------------

Reporter: andyr | Owner:
Type: optimization | Status: new
Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest

Resolution: | Keywords: Mac performance system font

Blocked By: | Blocking:
Patch: 1 |

---------------------------+-----------------------------------------

Comment (by vadz):

I completely forgot about `fontcache` in `wxFontRefData::Alloc()`, sorry.
I guess it would make sense to have a similar cache in
`wxFont(wxOSXSystemFont font)` ctor too.

It bothers me a bit that the existing cache is in `wxFontRefData` while
the new one would be in `wxFont`, but there shouldn't be any real
consequences of this.

There potentially could be problems due to using a local static variable,
rather than a global variable cleaned up by a `wxModule`, for the cache,
however...

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19191#comment:5>

[wxTrac]

#19191: Mac performance obtaining system fonts

---------------------------+-----------------------------------------

Reporter: andyr | Owner:
Type: optimization | Status: new
Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest

Resolution: | Keywords: Mac performance system font

Blocked By: | Blocking:
Patch: 1 |

---------------------------+-----------------------------------------

Comment (by csomor):

Replying to [comment:5 vadz]:

> It bothers me a bit that the existing cache is in `wxFontRefData` while
the new one would be in `wxFont`, but there shouldn't be any real
consequences of this.

we could move that code info `wxFontRefData` and hash
`wxCFRef<CTFontRefs>` against `CTFontUIFontType`, using std::map

> There potentially could be problems due to using a local static
variable, rather than a global variable cleaned up by a `wxModule`, for
the cache, however...

If I'd only use std::map as described above ?

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19191#comment:6>

[wxTrac]

#19191: Mac performance obtaining system fonts

---------------------------+-----------------------------------------

Reporter: andyr | Owner:
Type: optimization | Status: new
Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest

Resolution: | Keywords: Mac performance system font

Blocked By: | Blocking:
Patch: 1 |

---------------------------+-----------------------------------------

Comment (by csomor):

Hi Andy, could you please test with
https://github.com/wxWidgets/wxWidgets/pull/2497, I've placed the cache at
a lower-level, but I don't know wether all the other calls eat up a lot of
performance as well ... I'd have to move it up to the `wxFont` level then
...

Thanks,
Stefan

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19191#comment:7>

[wxTrac]

#19191: Mac performance obtaining system fonts

---------------------------+-----------------------------------------

Reporter: andyr | Owner:
Type: optimization | Status: new
Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest

Resolution: | Keywords: Mac performance system font

Blocked By: | Blocking:
Patch: 1 |

---------------------------+-----------------------------------------

Comment (by vadz):

Update: please check https://github.com/wxWidgets/wxWidgets/pull/2499
instead. TIA!

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19191#comment:8>

[wxTrac]

#19191: Mac performance obtaining system fonts

---------------------------+-----------------------------------------

Reporter: andyr | Owner:
Type: optimization | Status: new
Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest

Resolution: | Keywords: Mac performance system font

Blocked By: | Blocking:
Patch: 1 |

---------------------------+-----------------------------------------
Changes (by andyr):

* cc: andy@… (added)


Comment:

Yes this is great, amazing in fact. I've tried pausing my app repeatedly
in Xcode and haven't caught it in wxSystemSettingsNative::GetFont even
once.

But the main thing is, my current release build based on a 2021-01-19
snapshot of wx, with my static-font edit to
wxSystemSettingsNative::GetFont, shows 37% CPU in Activity Monitor when
playing and scrolling.

But with this fix it shows 23% CPU doing exactly the same thing.

So in other words the improvement is much greater than my static-font edit
achieved. I'm guessing this might have to do with other font usage of my
app that doesn't involve wxSystemSettingsNative::GetFont.

Many thanks!

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19191#comment:9>

[wxTrac]

#19191: Mac performance obtaining system fonts

---------------------------+-----------------------------------------

Reporter: andyr | Owner:
Type: optimization | Status: new
Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest

Resolution: | Keywords: Mac performance system font

Blocked By: | Blocking:
Patch: 1 |

---------------------------+-----------------------------------------

Comment (by vadz):

Thanks for testing!

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19191#comment:10>

[wxTrac]

#19191: Mac performance obtaining system fonts

---------------------------+-----------------------------------------
Reporter: andyr | Owner: Vadim Zeitlin <vadim@…>
Type: optimization | Status: closed

Priority: normal | Milestone:
Component: wxOSX | Version: dev-latest

Resolution: fixed | Keywords: Mac performance system font

Blocked By: | Blocking:
Patch: 1 |

---------------------------+-----------------------------------------
Changes (by Vadim Zeitlin <vadim@…>):

* status: new => closed
* owner: => Vadim Zeitlin <vadim@…>
* resolution: => fixed


Comment:

In [changeset:"27047eb338a3040cc23318c786857cd1f3f4ebd5/git-wxWidgets"
27047eb33/git-wxWidgets]:
{{{
#!CommitTicketReference repository="git-wxWidgets"
revision="27047eb338a3040cc23318c786857cd1f3f4ebd5"
Cache system fonts in wxOSX

Call CTFontCreateUIFontForLanguage() only once for every
CTFontUIFontType value and cache the results, as it is expensive to do
it every time wxSystemSettings::GetFont() is called.

Closes #19191.

This commit is best viewed ignoring whitespace-only changes.
}}}

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19191#comment:11>