Bug 70162 – Add deleteProperty to the MethodTable

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

[Mark Hahnenberg]

We now need to add both versions of deleteProperty to the MethodTable in ClassInfo in preparation from removing the virtual version of deleteProperty.

[Mark Hahnenberg]

Created attachment 111834 [details]
Patch

[Sam Weinig]

Have you done any back of the napkin calculations on what the memory difference between C++ vtables and the custom method tables are?

[Mark Hahnenberg]

(In reply to comment #2)
> Have you done any back of the napkin calculations on what the memory difference between C++ vtables and the custom method tables are?

I haven't, but my sense is that almost all objects share very few ClassInfos (and therefore MethodTables).  However, memory savings aren't the only reason we're trying to get rid of C++ virtual methods; mostly we just want more freedom in how we lay out our objects.

[Sam Weinig]

(In reply to comment #3)
> (In reply to comment #2)
> > Have you done any back of the napkin calculations on what the memory difference between C++ vtables and the custom method tables are?
> 
> I haven't, but my sense is that almost all objects share very few ClassInfos (and therefore MethodTables).  However, memory savings aren't the only reason we're trying to get rid of C++ virtual methods; mostly we just want more freedom in how we lay out our objects.

I didn't think memory savings was the impetuous, I am just a bit worried about the potential for bloat, so if the new mechanism uses more memory than the old one, we should understand how much.

[WebKit Review Bot]

Comment on attachment 111834 [details]
Patch

Rejecting attachment 111834 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
Source/JavaScriptCore/runtime/ClassInfo.h
Hunk #1 FAILED at 40.
1 out of 1 hunk FAILED -- saving rejects to file Source/JavaScriptCore/runtime/ClassInfo.h.rej
patching file Source/JavaScriptCore/runtime/JSFunction.h
Hunk #1 succeeded at 138 with fuzz 2 (offset 5 lines).
Hunk #2 succeeded at 148 with fuzz 2 (offset 2 lines).
patching file Source/WebCore/WebCore.exp.in

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Sam Weinig', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/10199577

[Geoffrey Garen]

> I didn't think memory savings was the impetuous, I am just a bit worried about the potential for bloat, so if the new mechanism uses more memory than the old one, we should understand how much.

We used to have one vtable per JS-derived class. When we're done, we'll lose the vtables, but gain one MethodTable per JS-derived class. I believe this will be a pure wash.

On Windows, one MethodTable per class will be a slight win, since Windows sometimes creates more than one vtable for the same class (e.g., when the class is used in two different DLLs).

Using plain function pointers instead of the member function pointers found in a vtable may also be a slight win, since C++ compilers sometimes encode member function pointers as more than just one pointer in size, and sometimes produce more than one copy of a given virtual function for a given class (e.g., a non-virtual thunk that inlines its target). But I don't think many JSCell subclasses trigger those mechanisms.

> my sense is that almost all objects share very few ClassInfos (and therefore MethodTables)

This is true, but also true of vtables. I think Sam's real question was about the total size of all MethodTables for all classes, not just the runtime footprint of commonly used MethodTables.

[Mark Hahnenberg]

Committed r98205: <http://trac.webkit.org/changeset/98205>