Bug 142941

Summary: GCTimer should work correctly when invoked multiple times during the same collection
Product: WebKit Reporter: Mark Hahnenberg <mhahnenb>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenb>
Status: NEW ---    
Severity: Normal CC: fpizlo, ggaren, kling, mark.lam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 140774    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch mark.lam: review+

Description Mark Hahnenberg 2015-03-21 17:24:28 PDT 
It would be nice to be able to record more fine-grained GC phases during phases that can occur multiple times per collection. For example, visiting live weak handles consists of a number of sub-phases that can be repeated until nothing changes. The current GCTimer implementation doesn't handle this correctly.
Comment 1 Mark Hahnenberg 2015-03-21 17:40:32 PDT 
Created attachment 249181 [details]
Patch
Comment 2 Mark Hahnenberg 2015-03-22 13:38:54 PDT 
Created attachment 249207 [details]
Patch
Comment 3 Geoffrey Garen 2015-03-23 11:00:41 PDT 
Comment on attachment 249207 [details]
Patch

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

r=me

> Source/JavaScriptCore/heap/Heap.cpp:83
> +struct TimeRecord {

Can you make this a real class in its own file?

I think we're supposed to use m_ naming for fields.
Comment 4 Mark Hahnenberg 2015-03-23 11:30:52 PDT 
(In reply to comment #3)
> Comment on attachment 249207 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249207&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/heap/Heap.cpp:83
> > +struct TimeRecord {
> 
> Can you make this a real class in its own file?
Sure, will do.

> 
> I think we're supposed to use m_ naming for fields.

Darin requested no "m_" for struct members on a previous patch dealing with GCTimer. Who do I believe!? :-)
Comment 5 Geoffrey Garen 2015-03-23 13:33:11 PDT 
I don't know what the common style is for structs, but if you make it a class, I know what the style is :).
Comment 6 Mark Hahnenberg 2015-03-28 17:02:32 PDT 
Created attachment 249674 [details]
Patch
Comment 7 Mark Hahnenberg 2015-03-28 17:03:18 PDT 
Split GCTimer and friends out into separate files and moved them over to normal classes w/ accessors.
Comment 8 Mark Hahnenberg 2015-03-28 17:22:13 PDT 
Created attachment 249675 [details]
Patch
Comment 9 Mark Lam 2015-03-31 10:12:34 PDT 
Comment on attachment 249675 [details]
Patch

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

r=me

> Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:1806
> -</Project>
>  \ No newline at end of file
> +</Project>

Can you fix this?  Seems like an unintended change.

> Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:4433
> -</Project>
>  \ No newline at end of file
> +</Project>

Another seemingly unintentional change.