Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't call FontCache invalidation callbacks when clearing FontCaches in response to memory pressure #10117

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

heycam
Copy link
Contributor

@heycam heycam commented Feb 15, 2023

639f82d

Don't call FontCache invalidation callbacks when clearing FontCaches in response to memory pressure
https://bugs.webkit.org/show_bug.cgi?id=251560
<rdar://problem/105197603>

Reviewed by NOBODY (OOPS!).

As of https://bugs.webkit.org/show_bug.cgi?id=249561, we call
FontCache::invalidate in response to memory pressure, in order to clear
some cache data. But we can end up calling FontCache invalidation
callbacks and FontSelector invalidation callbacks, both of which cause a
full style rebuild. This is not necessary when the FontCache is being
invalidated due to memory pressure.

We thread through the ShouldRunInvalidationCallbacks value through to
individual FontCache::invalidate calls so that we can skip informing the
FontSelectors about the invalidation.

The ShouldRunInvalidationCallbacks argument is made non-default, to
avoid future misuse.

* Source/WebCore/platform/ThreadGlobalData.cpp:
(WebCore::ThreadGlobalData::destroy):
* Source/WebCore/platform/graphics/FontCache.cpp:
(WebCore::FontCache::invalidate):
(WebCore::FontCache::invalidateAllFontCaches):
* Source/WebCore/platform/graphics/FontCache.h:
* Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:
(WebCore::fontCacheRegisteredFontsChangedNotificationCallback):
(WebCore::FontCache::platformReleaseNoncriticalMemory):
* Source/WebCore/testing/InternalSettings.cpp:
(WebCore::InternalSettings::setShouldMockBoldSystemFontForAccessibility):
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::invalidateFontCache):
* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::contentSizeCategoryDidChange):
* Source/WebKit/WebProcess/WebProcess.cpp:
(WebKit::WebProcess::terminate):
* Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:
(WebKit::WebProcess::accessibilityPreferencesDidChange):

639f82d

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  watch-sim

@heycam heycam self-assigned this Feb 15, 2023
@heycam heycam added the Text For bugs in text layout and rendering, including international text support. label Feb 15, 2023
…in response to memory pressure

https://bugs.webkit.org/show_bug.cgi?id=251560
<rdar://problem/105197603>

Reviewed by NOBODY (OOPS!).

As of https://bugs.webkit.org/show_bug.cgi?id=249561, we call
FontCache::invalidate in response to memory pressure, in order to clear
some cache data. But we can end up calling FontCache invalidation
callbacks and FontSelector invalidation callbacks, both of which cause a
full style rebuild. This is not necessary when the FontCache is being
invalidated due to memory pressure.

We thread through the ShouldRunInvalidationCallbacks value through to
individual FontCache::invalidate calls so that we can skip informing the
FontSelectors about the invalidation.

The ShouldRunInvalidationCallbacks argument is made non-default, to
avoid future misuse.

* Source/WebCore/platform/ThreadGlobalData.cpp:
(WebCore::ThreadGlobalData::destroy):
* Source/WebCore/platform/graphics/FontCache.cpp:
(WebCore::FontCache::invalidate):
(WebCore::FontCache::invalidateAllFontCaches):
* Source/WebCore/platform/graphics/FontCache.h:
* Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:
(WebCore::fontCacheRegisteredFontsChangedNotificationCallback):
(WebCore::FontCache::platformReleaseNoncriticalMemory):
* Source/WebCore/testing/InternalSettings.cpp:
(WebCore::InternalSettings::setShouldMockBoldSystemFontForAccessibility):
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::invalidateFontCache):
* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::contentSizeCategoryDidChange):
* Source/WebKit/WebProcess/WebProcess.cpp:
(WebKit::WebProcess::terminate):
* Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:
(WebKit::WebProcess::accessibilityPreferencesDidChange):
@heycam heycam force-pushed the font-cache-invalidate-callback branch from 1b29d3a to 639f82d Compare February 15, 2023 04:36
@heycam heycam marked this pull request as ready for review February 15, 2023 06:44
@heycam heycam requested a review from cdumez as a code owner February 15, 2023 06:44
@heycam heycam requested a review from litherum February 15, 2023 06:44
@heycam
Copy link
Contributor Author

heycam commented Feb 16, 2023

This is not having the memory impact I'd expect. Back to draft.

@heycam heycam marked this pull request as draft February 16, 2023 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Text For bugs in text layout and rendering, including international text support.
Projects
None yet
2 participants