Bug 121045

Summary: CopiedSpace::startedCopying should not call MarkedSpace::capacity
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 121074    
Attachments:
Description Flags
Patch
none
Patch ggaren: review+

Mark Hahnenberg
Reported 2013-09-09 13:51:27 PDT
MarkedSpace::capacity() iterates every block in MarkedSpace. Heap has m_totalBytesVisited, which is a good enough approximation of the size of MarkedSpace and it's O(1) so we should use it instead.
Attachments
Patch (1.51 KB, patch)
2013-09-09 13:52 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Patch (2.95 KB, patch)
2013-09-09 17:13 PDT, Mark Hahnenberg 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Hahnenberg
Comment 1 2013-09-09 13:52:38 PDT
Created attachment 211079 [details] Patch
Mark Hahnenberg
Comment 2 2013-09-09 17:13:21 PDT
Created attachment 211123 [details] Patch
Mark Hahnenberg
Comment 3 2013-09-09 17:14:45 PDT
I was getting a small but persistent slow down on kraken with the first version, so I tried just keeping track as we add and remove MarkedBlocks from the MarkedSpace. This version didn't show any slowdown.
Geoffrey Garen
Comment 4 2013-09-09 17:17:35 PDT
Comment on attachment 211123 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=211123&action=review r=me > Source/JavaScriptCore/heap/MarkedSpace.h:135 > + size_t m_totalCapacity; I think "capacity" is a slightly better name here. We don't have a concept of "partial capacity" in this class.
Mark Hahnenberg
Comment 5 2013-09-09 17:34:19 PDT
Committed r155406: <http://trac.webkit.org/changeset/155406>
Note You need to log in before you can comment on or make changes to this bug.