Bug 100702 – [chromium] Defer commits between page unload and first invalidation in threaded compositing mode

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Robinson]

[chromium] Defer commits between page unload and first invalidation in threaded compositing mode

[James Robinson]

Created attachment 171320 [details]
Patch

[James Robinson]

FYI - I'm still validating the performance + correctness.  Sadly this is not something we have great test coverage for.

[James Robinson]

Seems to do the trick.  Perf #s from my linux box (where slower-than-average texture upload speeds are having a disproportionately bad impact on threaded mode perf), all average of 3 runs:

moz software: 6014
moz FCM: 6295 (4.7% slower than software)
moz FCM threaded unpatched: 7535 (25.3% slower than software)
moz FCM threaded with this patch: 6359 (5.7% slower than software)

intl1 software: 27035
intl1 FCM: 28458 (5.3% slower than software)
intl1 FCM threaded unpatched: 41045 (51.8% slower than software)
intl1 FCM threaded with this patch: 29382 (8.7% slower than software)

[Adrienne Walker]

Comment on attachment 171320 [details]
Patch

R=me.  I wish we had better test coverage too, but this looks good to me.

[WebKit Review Bot]

Comment on attachment 171320 [details]
Patch

Attachment 171320 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14627302

New failing tests:
http/tests/inspector/appcache/appcache-iframe-manifests.html
http/tests/inspector/network/cached-resource-destroyed-too-big-discarded.html
http/tests/inspector/indexeddb/database-data.html
http/tests/inspector/network/har-content.html
http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html
http/tests/inspector/filesystem/delete-entry.html
http/tests/inspector/network/network-disable-cache-xhrs.html
http/tests/appcache/crash-when-navigating-away-then-back.html
http/tests/inspector-enabled/injected-script-discard.html
http/tests/inspector/network/network-initiator-from-console.html
http/tests/inspector/indexeddb/database-structure.html
http/tests/inspector/appcache/appcache-swap.html
http/tests/inspector/indexeddb/resources-panel.html
http/tests/inspector/filesystem/request-file-content.html
http/tests/inspector/network/network-cachedresources-with-same-urls.html
http/tests/inspector/network/async-xhr-json-mime-type.html
http/tests/inspector/network/network-disable-cache-memory.html
http/tests/inspector/network/network-iframe-load-and-delete.html
http/tests/inspector/filesystem/request-metadata.html
http/tests/inspector/filesystem/request-directory-content.html
http/tests/inspector/network/network-content-replacement-xhr.html
http/tests/inspector/network/network-image-404.html
http/tests/inspector/filesystem/directory-tree.html
http/tests/inspector/indexeddb/database-names.html
http/tests/inspector/network/network-disabling-check-no-memory-leak.html
http/tests/inspector/network/cached-resource-destroyed-moved-to-storage.html
http/tests/inspector/network/image-as-text-loading-data-url.html
http/tests/inspector/network/network-empty-xhr.html
http/tests/inspector/network/network-cyrillic-xhr.html
http/tests/inspector/filesystem/request-filesystem-root.html

[James Robinson]

(In reply to comment #5)
> (From update of attachment 171320 [details])
> Attachment 171320 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/14627302
> 
> New failing tests:
> http/tests/inspector/appcache/appcache-iframe-manifests.html
> http/tests/inspector/network/cached-resource-destroyed-too-big-discarded.html
> http/tests/inspector/indexeddb/database-data.html
> http/tests/inspector/network/har-content.html
> http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html
> http/tests/inspector/filesystem/delete-entry.html
> http/tests/inspector/network/network-disable-cache-xhrs.html
> http/tests/appcache/crash-when-navigating-away-then-back.html
> http/tests/inspector-enabled/injected-script-discard.html
> http/tests/inspector/network/network-initiator-from-console.html
> http/tests/inspector/indexeddb/database-structure.html
> http/tests/inspector/appcache/appcache-swap.html
> http/tests/inspector/indexeddb/resources-panel.html
> http/tests/inspector/filesystem/request-file-content.html
> http/tests/inspector/network/network-cachedresources-with-same-urls.html
> http/tests/inspector/network/async-xhr-json-mime-type.html
> http/tests/inspector/network/network-disable-cache-memory.html
> http/tests/inspector/network/network-iframe-load-and-delete.html
> http/tests/inspector/filesystem/request-metadata.html
> http/tests/inspector/filesystem/request-directory-content.html
> http/tests/inspector/network/network-content-replacement-xhr.html
> http/tests/inspector/network/network-image-404.html
> http/tests/inspector/filesystem/directory-tree.html
> http/tests/inspector/indexeddb/database-names.html
> http/tests/inspector/network/network-disabling-check-no-memory-leak.html
> http/tests/inspector/network/cached-resource-destroyed-moved-to-storage.html
> http/tests/inspector/network/image-as-text-loading-data-url.html
> http/tests/inspector/network/network-empty-xhr.html
> http/tests/inspector/network/network-cyrillic-xhr.html
> http/tests/inspector/filesystem/request-filesystem-root.html

Ran these locally and they passed, highly unlikely the failure is related.

[James Robinson]

Comment on attachment 171320 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=171320&action=review

> Source/WebKit/chromium/src/WebViewImpl.cpp:3835
> +    if (m_layerTreeViewCommitsDeferred) {

oh snap, i'm not initializing this member variable. whoops

[James Robinson]

Committed r132862: <http://trac.webkit.org/changeset/132862>