Bug 142941 – GCTimer should work correctly when invoked multiple times during the same collection

• Status: NEW
• Component: JavaScriptCore

[Mark Hahnenberg]

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.

[Mark Hahnenberg]

Created attachment 249181 [details]
Patch

[Mark Hahnenberg]

Created attachment 249207 [details]
Patch

[Geoffrey Garen]

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.

[Mark Hahnenberg]

(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!? :-)

[Geoffrey Garen]

I don't know what the common style is for structs, but if you make it a class, I know what the style is :).

[Mark Hahnenberg]

Created attachment 249674 [details]
Patch

[Mark Hahnenberg]

Split GCTimer and friends out into separate files and moved them over to normal classes w/ accessors.

[Mark Hahnenberg]

Created attachment 249675 [details]
Patch

[Mark Lam]

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.