Bug 74331

Summary: De-virtualize destructors
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, gustavo.noronha, gustavo, japhet, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 74332, 74333, 74335, 74336, 74338, 74339, 74341, 74342, 74347, 74348, 74349, 74350, 74353, 74355    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
ggaren: review+, webkit.review.bot: commit-queue-
Bencher results none

Description Mark Hahnenberg 2011-12-12 14:21:25 PST 
In order to remove the virtual destructors, which are the last of the virtual functions, from the JSCell hierarchy, we need to add the ClassInfo pointer to the cell rather than to the structure because in order to be able to lazily call the static destroy() functions that will replace the virtual destructors, we need to be able to access the ClassInfo without the danger of the object's Structure being collected before the object itself.

This is a tracking bug for that effort.  Each of the patches will be put up individually for review, but they will all be landed together as one giant patch to avoid any performance regressions (I also only verified that each patch would build on 64-bit Mac, and later went back and stabilized the other platforms).
Comment 1 Mark Hahnenberg 2011-12-13 15:02:46 PST 
Created attachment 119091 [details]
Patch
Comment 2 Mark Hahnenberg 2011-12-13 17:02:17 PST 
Created attachment 119112 [details]
Patch
Comment 3 Mark Hahnenberg 2011-12-13 17:04:59 PST 
Comment on attachment 119112 [details]
Patch

This patch is obviously more than just de-virtualizing destructors.  Destructors were the last virtual function in the JSCell hierarchy, so their removal caused a cascade of other necessary changes to be done in the same patch.
Comment 4 WebKit Review Bot 2011-12-13 17:08:26 PST 
Attachment 119112 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/JavaScriptCore/API/JS..." exit_code: 1

Source/JavaScriptCore/jit/JITOpcodes.cpp:52:  string_failureCases2 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/jit/JITOpcodes32_64.cpp:55:  string_failureCases2 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 2 in 144 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Early Warning System Bot 2011-12-13 17:41:20 PST 
Comment on attachment 119112 [details]
Patch

Attachment 119112 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10876038
Comment 6 Gyuyoung Kim 2011-12-13 18:06:54 PST 
Comment on attachment 119112 [details]
Patch

Attachment 119112 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10876045
Comment 7 Gustavo Noronha (kov) 2011-12-13 18:54:21 PST 
Comment on attachment 119112 [details]
Patch

Attachment 119112 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10873050
Comment 8 Collabora GTK+ EWS bot 2011-12-13 23:11:39 PST 
Comment on attachment 119112 [details]
Patch

Attachment 119112 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10875113
Comment 9 Mark Hahnenberg 2011-12-14 11:28:56 PST 
Created attachment 119263 [details]
Patch
Comment 10 WebKit Review Bot 2011-12-14 11:32:27 PST 
Attachment 119263 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/JavaScriptCore/API/JS..." exit_code: 1

Source/JavaScriptCore/jit/JITOpcodes.cpp:52:  string_failureCases2 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/jit/JITOpcodes32_64.cpp:55:  string_failureCases2 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 2 in 152 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Geoffrey Garen 2011-12-14 11:39:07 PST 
Comment on attachment 119263 [details]
Patch

Since I've already reviewed the sub-patches, r=me if it builds.
Comment 12 Early Warning System Bot 2011-12-14 11:45:54 PST 
Comment on attachment 119263 [details]
Patch

Attachment 119263 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10872346
Comment 13 Gyuyoung Kim 2011-12-14 12:20:46 PST 
Comment on attachment 119263 [details]
Patch

Attachment 119263 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10872357
Comment 14 Gustavo Noronha (kov) 2011-12-14 13:08:42 PST 
Comment on attachment 119263 [details]
Patch

Attachment 119263 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10873360
Comment 15 Gustavo Noronha (kov) 2011-12-14 15:38:10 PST 
Comment on attachment 119263 [details]
Patch

Attachment 119263 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10870456
Comment 16 Mark Hahnenberg 2011-12-14 16:11:51 PST 
Created attachment 119317 [details]
Patch
Comment 17 WebKit Review Bot 2011-12-14 16:16:44 PST 
Attachment 119317 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/JavaScriptCore/API/JS..." exit_code: 1

Source/JavaScriptCore/jit/JITOpcodes.cpp:52:  string_failureCases2 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/jit/JITOpcodes32_64.cpp:55:  string_failureCases2 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 2 in 152 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 Gyuyoung Kim 2011-12-14 16:56:00 PST 
Comment on attachment 119317 [details]
Patch

Attachment 119317 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10905021
Comment 19 Mark Hahnenberg 2011-12-14 21:27:56 PST 
Created attachment 119371 [details]
Patch
Comment 20 WebKit Review Bot 2011-12-14 21:30:30 PST 
Attachment 119371 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/JavaScriptCore/API/JS..." exit_code: 1

Source/JavaScriptCore/jit/JITOpcodes.cpp:52:  string_failureCases2 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/jit/JITOpcodes32_64.cpp:55:  string_failureCases2 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 2 in 152 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 Geoffrey Garen 2011-12-15 18:47:11 PST 
Comment on attachment 119371 [details]
Patch

r=me if it builds.
Comment 22 WebKit Review Bot 2011-12-15 20:21:20 PST 
Comment on attachment 119371 [details]
Patch

Rejecting attachment 119371 [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:
/Netscape/JSNPMethod.cpp
patching file Source/WebKit2/WebProcess/Plugins/Netscape/JSNPObject.cpp
patching file Source/WebKit2/WebProcess/Plugins/Netscape/JSNPObject.h
patching file Source/WebKit2/win/WebKit2.def
patching file Source/WebKit2/win/WebKit2CFLite.def
patching file Source/autotools/symbols.filter
patching file ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Geoffrey Garen', u'--f..." exit_code: 1

Full output: http://queues.webkit.org/results/10914511
Comment 23 Mark Hahnenberg 2011-12-16 11:13:08 PST 
Committed r103083: <http://trac.webkit.org/changeset/103083>
Comment 24 Mark Hahnenberg 2011-12-16 11:22:54 PST 
Created attachment 119639 [details]
Bencher results

Performance results for this patch.  Mostly a wash, small win on kraken.