Qt accelerating compositing uses QPixmapCache. When web site has a big accelerating compositing layer on devices with low memory we can reach the QPixmapCache limit very fast. In this case pixmap is dropped from QPixmapCache and getting re-allocated on every paint. This makes performance very bad. Increasing QPixmapCache limit wouldn't help in this case since we run out of graphics memory. In the same time those html layers are quite simple and render fast without any cache on graphics accelerated hardware. I propose to disable html layer cache in this case. It will still allow to remove dynamic content from tiled backing store with AC and keep AC memory requirements low.
Created attachment 83172 [details] patch to address the issue Patch to address the issue with hight memory usage for AC
Attachment 83172 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:210: The parameter name "painter" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:210: The parameter name "region" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:648: An else should appear on the same line as the preceding } [whitespace/newline] [4] Source/WebKit/qt/Api/qwebpage.cpp:1177: An else should appear on the same line as the preceding } [whitespace/newline] [4] Source/WebKit/qt/ChangeLog:6: Line contains tab character. [whitespace/tab] [5] Total errors found: 5 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 83172 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7935953
Attachment 83172 [details] did not build on qt: Build output: http://queues.webkit.org/results/7935954
Attachment 83172 [details] did not build on win: Build output: http://queues.webkit.org/results/7938872
Created attachment 83188 [details] addressed style and build issues
Comment on attachment 83188 [details] addressed style and build issues This patch contains only ChangeLog.
What is the point of accelerated compositing in this case? What do you think is "accelerated"?
(In reply to comment #8) > What is the point of accelerated compositing in this case? What do you think is "accelerated"? I think the point here is that the AC layers are not tiled.
(In reply to comment #0) > Qt accelerating compositing uses QPixmapCache. When web site has a big accelerating compositing layer on devices with low memory we can reach the QPixmapCache limit very fast. In this case pixmap is dropped from QPixmapCache and getting re-allocated on every paint. This makes performance very bad. > Increasing QPixmapCache limit wouldn't help in this case since we run out of graphics memory. In the same time those html layers are quite simple and render fast without any cache on graphics accelerated hardware. Which HTML layers? Which websites? Which platform? Does this mean that 3D & animations run well on this platform even when this cache is turned off? (which effectively makes AC unaccelerated)
(In reply to comment #10) > (In reply to comment #0) > > Qt accelerating compositing uses QPixmapCache. When web site has a big accelerating compositing layer on devices with low memory we can reach the QPixmapCache limit very fast. In this case pixmap is dropped from QPixmapCache and getting re-allocated on every paint. This makes performance very bad. > > Increasing QPixmapCache limit wouldn't help in this case since we run out of graphics memory. In the same time those html layers are quite simple and render fast without any cache on graphics accelerated hardware. > > Which HTML layers? GraphicsLayerQtImpl::HTMLContentType > Which websites? For example, iphone versions of google maps, or iphone gmail. > Which platform? Any platform with low RAM (CPU or GPU). > Does this mean that 3D & animations run well on this platform even when this cache is turned off? (which effectively makes AC unaccelerated) This is totally unrelated. 3D, animations, media are layers of different type. They do not have cache by default and not affected by this. This patch about html layers which have full size QPixmap cache by default or create another tiled layer and cause RAM overhead which is quite big.
(In reply to comment #8) > What is the point of accelerated compositing in this case? Removing dynamic content from tiled backing store. > What do you think is "accelerated"? Animated images, plugins and etc. Just everything as it is, but html layers will not have pixmap cache.
Created attachment 83315 [details] actual patch
Attachment 83315 [details] did not build on win: Build output: http://queues.webkit.org/results/7942327
Created attachment 83327 [details] win build fix
Attachment 83327 [details] did not build on win: Build output: http://queues.webkit.org/results/7947244
Created attachment 83375 [details] new patch As discussed on irc changes are kept only in GraphicsLayerQt.cpp as they are Qt specific. If QPixmapCache limit is two small (2048Kb) them QPixmapCache is not used when rendering HTMLContentType layers.
(In reply to comment #17) > Created an attachment (id=83375) [details] > new patch LGTM, though I'm not a reviewer.
Comment on attachment 83375 [details] new patch View in context: https://bugs.webkit.org/attachment.cgi?id=83375&action=review > Source/WebCore/ChangeLog:5 > + [Qt] Dont use QPixmapCache if QPixmapCache::cacheLimit() is too small Double spaces after 'if'. > Source/WebCore/ChangeLog:6 > +(2048Kb for now). Should be indented with the line above. > Source/WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:57 > +#define PIXMAP_CACHE_USE_THRESHOLD 2048 We try to avoid using #define for constants. static const int gMinimumPixmapCacheLimit = 2048; > Source/WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:213 > + void drawLayerContent(QPainter&, QRegion&); QPainter& -> QPainter* > Source/WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:649 > + QRegion region(option->exposedRect.toRect()); > + drawLayerContent(*painter, region); We shouldn't be clipping to a region if we know it's always a rect. Change drawLayerContent() to take a const QRect& instead. > Source/WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:651 > + Unnecessary empty line.
Created attachment 83493 [details] Fixed style and clip region Addressed issues in Comment #19
Comment on attachment 83493 [details] Fixed style and clip region r=me
Comment on attachment 83493 [details] Fixed style and clip region Clearing flags on attachment: 83493 Committed r79595: <http://trac.webkit.org/changeset/79595>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/79595 might have broken GTK Linux 32-bit Release
Revision r79595 cherry-picked into qtwebkit-2.1.x with commit 12a2bcc <http://gitorious.org/webkit/qtwebkit/commit/12a2bcc>