WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED DUPLICATE of
bug 227229
227277
REGRESSION(
r279104
): [GPU Process] Canvas layout tests became flaky
https://bugs.webkit.org/show_bug.cgi?id=227277
Summary
REGRESSION(r279104): [GPU Process] Canvas layout tests became flaky
Said Abou-Hallawa
Reported
2021-06-22 21:37:51 PDT
The change
r279104
tried to fix some issues in RemoteResourceCacheProxy::didFinalizeRenderingUpdate(). It fixed the early return from: if (unusedFontCount < minimumFractionOfUnusedFontCountToTriggerRemoval * totalFontCount && unusedFontCount <= maximumUnusedFontCountToSkipRemoval) return; // which is equivalent to the following code because unusedFontCount and maximumUnusedFontCountToSkipRemoval are both unsigned and maximumUnusedFontCountToSkipRemoval = 0. // if (!unusedFontCount) // return; to: if (unusedFontCount <= minimumFractionOfUnusedFontCountToTriggerRemoval * totalFontCount) { prepareForNextRenderingUpdate(); return; } And it fixed the synchronization of the cached fonts between WebProcess and GPUP: from: // This code removes the font object from GPUP but keeps the font identifier in WebProcess for (auto& item : m_fontIdentifierToLastRenderingUpdateVersionMap) { if (m_currentRenderingUpdateVersion - item.value >= minimumRenderingUpdateCountToKeepFontAlive) m_remoteRenderingBackendProxy.releaseRemoteResource(item.key); } to: m_fontIdentifierToLastRenderingUpdateVersionMap.removeIf([&] (const auto& item) { if (m_currentRenderingUpdateVersion - item.value < minimumRenderingUpdateCountToKeepFontAlive) return false; m_remoteRenderingBackendProxy.releaseRemoteResource(item.key); return true; }); But when I looked closely at the original RemoteResourceCacheProxy::didFinalizeRenderingUpdate() before
r279104
, I found it was always making an early return and basically it was doing nothing. Here is the explanation. 1. When the fonts are cached for the first RenderingUpdate to a canvas, all the entries in m_fontIdentifierToLastRenderingUpdateVersionMap will have value = 1 since m_currentRenderingUpdateVersion is initialized with 1. 2. m_numberOfFontsUsedInCurrentRenderingUpdate will be equal to the number of fonts cached during the first RenderingUpdate. 3. RemoteResourceCacheProxy::didFinalizeRenderingUpdate(), will find unusedFontCount == 0 because m_numberOfFontsUsedInCurrentRenderingUpdate == m_fontIdentifierToLastRenderingUpdateVersionMap.size(). So it will return without changing m_numberOfFontsUsedInCurrentRenderingUpdate or m_currentRenderingUpdateVersion. 4. If in the next RenderingUpdate a new font is cached, it will inherit m_currentRenderingUpdateVersion which is still 1. And m_numberOfFontsUsedInCurrentRenderingUpdate will be incremented by 1. 5. Caching more fonts will increment m_numberOfFontsUsedInCurrentRenderingUpdate as if all the cached fonts were used for m_currentRenderingUpdateVersion = 1. 6. RemoteResourceCacheProxy::didFinalizeRenderingUpdate() will make an early return again because unusedFontCount is always zero. So the change
r279104
exercises a new code path which was never exercised before. Removing the font from the cache in GPUP and recaching it causes RemoteRenderingBackend to wakeUpAndApplyDisplayList() more often if the Font was missing in the GPUP cache. I think the solution is to remove RemoteResourceCacheProxy::didFinalizeRenderingUpdate() since it has been doing nothing since it was written.
Attachments
Patch
(7.25 KB, patch)
2021-06-22 21:45 PDT
,
Said Abou-Hallawa
no flags
Details
Formatted Diff
Diff
Patch
(7.25 KB, patch)
2021-06-22 22:43 PDT
,
Said Abou-Hallawa
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(7.25 KB, patch)
2021-06-22 23:04 PDT
,
Said Abou-Hallawa
no flags
Details
Formatted Diff
Diff
Patch
(7.25 KB, patch)
2021-06-22 23:07 PDT
,
Said Abou-Hallawa
no flags
Details
Formatted Diff
Diff
Patch
(7.25 KB, patch)
2021-06-22 23:08 PDT
,
Said Abou-Hallawa
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Said Abou-Hallawa
Comment 1
2021-06-22 21:45:48 PDT
Created
attachment 432017
[details]
Patch
Said Abou-Hallawa
Comment 2
2021-06-22 22:43:11 PDT
Created
attachment 432019
[details]
Patch
Said Abou-Hallawa
Comment 3
2021-06-22 23:04:31 PDT
Created
attachment 432020
[details]
Patch
Said Abou-Hallawa
Comment 4
2021-06-22 23:07:34 PDT
Created
attachment 432021
[details]
Patch
Said Abou-Hallawa
Comment 5
2021-06-22 23:08:10 PDT
Created
attachment 432022
[details]
Patch
Said Abou-Hallawa
Comment 6
2021-06-23 10:36:21 PDT
*** This bug has been marked as a duplicate of
bug 227229
***
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug