Bug 70162 - Add deleteProperty to the MethodTable
Summary: Add deleteProperty to the MethodTable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks: 67690
  Show dependency treegraph
 
Reported: 2011-10-14 17:51 PDT by Mark Hahnenberg
Modified: 2011-10-23 21:22 PDT (History)
3 users (show)

See Also:


Attachments
Patch (5.93 KB, patch)
2011-10-20 13:15 PDT, Mark Hahnenberg
sam: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2011-10-14 17:51:56 PDT
We now need to add both versions of deleteProperty to the MethodTable in ClassInfo in preparation from removing the virtual version of deleteProperty.
Comment 1 Mark Hahnenberg 2011-10-20 13:15:30 PDT
Created attachment 111834 [details]
Patch
Comment 2 Sam Weinig 2011-10-23 12:04:35 PDT
Have you done any back of the napkin calculations on what the memory difference between C++ vtables and the custom method tables are?
Comment 3 Mark Hahnenberg 2011-10-23 12:18:24 PDT
(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.
Comment 4 Sam Weinig 2011-10-23 12:21:56 PDT
(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.
Comment 5 WebKit Review Bot 2011-10-23 12:23:43 PDT
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
Comment 6 Geoffrey Garen 2011-10-23 14:14:45 PDT
> 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.
Comment 7 Mark Hahnenberg 2011-10-23 18:45:25 PDT
Committed r98205: <http://trac.webkit.org/changeset/98205>