Bug 145817

Summary: GraphicsContext state stack wasting lots of memory when empty.
Product: WebKit Reporter: Andreas Kling <kling>
Component: CanvasAssignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ggaren, kling
Priority: P2 Keywords: Performance
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Andreas Kling
Reported 2015-06-09 14:33:27 PDT
GraphicsContext::restore() uses removeLast() which never nukes the Vector<State>'s backing store. This means that we always have capacity for at least 16 states in each GraphicsContext that has ever save()d. Since canvas elements have a persistent GraphicsContext, this adds up to a bunch of memory with multiple canvases around.
Attachments
Patch (2.19 KB, patch)
2015-06-09 14:53 PDT, Andreas Kling
no flags
Andreas Kling
Comment 1 2015-06-09 14:53:33 PDT
Geoffrey Garen
Comment 2 2015-06-09 15:14:17 PDT
Comment on attachment 254604 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254604&action=review r=me > Source/WebCore/platform/graphics/GraphicsContext.cpp:140 > + // Make sure we deallocate the state stack buffer when it goes empty. > + // Canvas elements will immediately save() again, but that goes into inline capacity. > + if (m_stack.isEmpty()) > + m_stack.clear(); Should Vector do this automatically?
Geoffrey Garen
Comment 3 2015-06-09 15:14:47 PDT
To elaborate, I really hate it that std::vector does not do this automatically. At the limit, you have to add a call to clear to every client!
Andreas Kling
Comment 4 2015-06-09 16:19:05 PDT
(In reply to comment #3) > To elaborate, I really hate it that std::vector does not do this > automatically. At the limit, you have to add a call to clear to every client! I guess you mean WTF::Vector. We can totes do that. Let's separate the issues though.
Geoffrey Garen
Comment 5 2015-06-09 16:32:41 PDT
> > To elaborate, I really hate it that std::vector does not do this > > automatically. At the limit, you have to add a call to clear to every client! > > I guess you mean WTF::Vector. > We can totes do that. Let's separate the issues though. No, I mean that std::vector never ever shrinks until you call shrink_to_fit, which is super annoying, while WTF::Vector often shrinks automatically, but misses in this case.
WebKit Commit Bot
Comment 6 2015-06-09 17:47:23 PDT
Comment on attachment 254604 [details] Patch Clearing flags on attachment: 254604 Committed r185396: <http://trac.webkit.org/changeset/185396>
WebKit Commit Bot
Comment 7 2015-06-09 17:47:26 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.