[Webkit-unassigned] [Bug 65901] The JSC JIT currently has no facility to profile and report the types of values

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=65901


Gavin Barraclough <barraclough at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #103335|review?                     |review-
               Flag|                            |




--- Comment #6 from Gavin Barraclough <barraclough at apple.com>  2011-08-17 14:45:20 PST ---
(From update of attachment 103335)
View in context: https://bugs.webkit.org/attachment.cgi?id=103335&action=review

r- for a bunch of small suggested changes, mostly just that ValueProfile really should be in its own header.

> Source/JavaScriptCore/bytecode/CodeBlock.h:207
> +#if ENABLE(VALUE_PROFILER)

The file CodeBlock is a bit too big already, and this struct is non-trivial, I think we really need to add a new ValueProfile.h header.

> Source/JavaScriptCore/bytecode/CodeBlock.h:682
> +        SegmentedVector<ValueProfile, sizeof(ValueProfile) * 8> m_valueProfiles;

This code is a little weird - I think the second argument to the template should be a count of ValueProfiles, not a size, so isn't this growing n^2 in the size of ValueProfile? - don't you just want 8 here?

> Source/JavaScriptCore/jit/JIT.cpp:377
> +    unsigned numberOfValueProfiles = m_codeBlock->numberOfValueProfiles();

Is this just asserting that no slow cases add any new (FirstProfilingSite) value profiling code? - I think an comment here would be good.

> Source/JavaScriptCore/jit/JIT.h:313
> +#endif

Are value / scratch ever passed? - I didn't see any case of this.
If not, it may make the code simpler to remove these arguments.
Also, this function may trample regT2/regT3 - I think it's worth commenting this on the method, to try to make this more apparent.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.