Bug 89400

Summary: GCActivityCallback and IncrementalSweeper should share code
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, gustavo, philn, rakuco, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 89516    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Checking builds
none
Checking builds none

Mark Hahnenberg
Reported 2012-06-18 16:07:08 PDT
A lot of functionality is duplicated between GCActivityCallback and IncrementalSweeper. We should extract the common functionality out into a separate class that both of them can inherit from. This refactoring will be an even greater boon when we add the ability to shut these two things down in a thread-safe fashion (see bug 89123).
Attachments
Patch (28.07 KB, patch)
2012-06-18 16:34 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Patch (28.95 KB, patch)
2012-06-18 17:23 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Patch (29.13 KB, patch)
2012-06-18 17:50 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Checking builds (28.60 KB, patch)
2012-06-19 11:09 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Checking builds (28.59 KB, patch)
2012-06-19 11:10 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Mark Hahnenberg
Comment 1 2012-06-18 16:34:08 PDT
Created attachment 148202 [details] Patch
Early Warning System Bot
Comment 2 2012-06-18 17:10:53 PDT
Comment on attachment 148202 [details] Patch Attachment 148202 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12982201
Build Bot
Comment 3 2012-06-18 17:13:36 PDT
Comment on attachment 148202 [details] Patch Attachment 148202 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12978393
Early Warning System Bot
Comment 4 2012-06-18 17:19:18 PDT
Comment on attachment 148202 [details] Patch Attachment 148202 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12969868
Mark Hahnenberg
Comment 5 2012-06-18 17:23:06 PDT
Created attachment 148207 [details] Patch
Mark Hahnenberg
Comment 6 2012-06-18 17:50:19 PDT
Created attachment 148216 [details] Patch
Geoffrey Garen
Comment 7 2012-06-18 19:48:42 PDT
Comment on attachment 148216 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=148216&action=review r=me > Source/JavaScriptCore/runtime/ExternalVMAgent.h:14 > +class ExternalVMAgent { This name seemed a bit vague to me. "External" isn't just says what you're not (i.e., not internal), and "VM" doesn't specifically convey "related to the GC heap". How about HeapTimer or ScheduledHeapAgent?
Mark Hahnenberg
Comment 8 2012-06-19 11:09:14 PDT
Created attachment 148363 [details] Checking builds
Mark Hahnenberg
Comment 9 2012-06-19 11:10:18 PDT
Created attachment 148364 [details] Checking builds
Mark Hahnenberg
Comment 10 2012-06-19 11:10:43 PDT
Re-uploading patch to make sure I didn't break any builds when renaming ExternalVMAgent to HeapTimer.
WebKit Review Bot
Comment 11 2012-06-19 12:17:50 PDT
Comment on attachment 148364 [details] Checking builds Clearing flags on attachment: 148364 Committed r120742: <http://trac.webkit.org/changeset/120742>
WebKit Review Bot
Comment 12 2012-06-19 12:17:55 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.