Bug 68567 – Add custom vtable struct to ClassInfo struct

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Hahnenberg]

We want to put a vtable in ClassInfo in preparation for moving virtual methods out of JSCell and into our own custom vtable representation.  To concisely generate this table in each file that defines its own ClassInfo struct (s_info), we could use a macro called CREATE_METHOD_TABLE that takes the name of the class as the argument and generates a statically declared struct that contains all of the pointers to the static methods that represent the virtual functions for that particular class.

In this patch, we will also include the first virtual method to be included in the table, visitChildren.

[Mark Hahnenberg]

Created attachment 108542 [details]
Patch

[WebKit Review Bot]

Comment on attachment 108542 [details]
Patch

Attachment 108542 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9842031

New failing tests:
svg/custom/svg-fonts-word-spacing.html

[Adam Barth]

> svg/custom/svg-fonts-word-spacing.html

Sorry, this test seems to be super flaky.

[Mark Hahnenberg]

(In reply to comment #3)
> > svg/custom/svg-fonts-word-spacing.html
> 
> Sorry, this test seems to be super flaky.

:-(  Thanks for letting me know it's (most likely) not my fault.

[Geoffrey Garen]

Comment on attachment 108542 [details]
Patch

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

r=me with the changes I suggested.

> Source/JavaScriptCore/runtime/ClassInfo.h:38
> +    struct VirtualMethodTable {
> +        typedef void (*VisitChildrenFunctionPtr)(JSCell*, SlotVisitor&);
> +        VisitChildrenFunctionPtr visitChildrenFunctionPtr;
> +    };
> +
> +#define CREATE_METHOD_TABLE(ClassName) { \

It is bad when one thing becomes two. You call this thing "virtual method table" and "method table". (Below, you also call it "vtable".) Let's call it exactly one thing everywhere. I like "method table".

> Source/JavaScriptCore/runtime/RegExpObject.h:85
>          virtual void visitChildrenVirtual(SlotVisitor&);
>          static void visitChildren(JSCell*, SlotVisitor&);

Why did these need to become public? This is the kind of thing that can benefit from a line-by-line ChangeLog comment. You should add one.

[Mark Hahnenberg]

Created attachment 108566 [details]
Fixing review issues

[Oliver Hunt]

Comment on attachment 108566 [details]
Fixing review issues

r=me

[WebKit Review Bot]

Comment on attachment 108566 [details]
Fixing review issues

Clearing flags on attachment: 108566

Committed r95936: <http://trac.webkit.org/changeset/95936>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.