Bug 74331 – De-virtualize destructors

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

[Mark Hahnenberg]

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).

[Mark Hahnenberg]

Created attachment 119091 [details]
Patch

[Mark Hahnenberg]

Created attachment 119112 [details]
Patch

[Mark Hahnenberg]

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.

[WebKit Review Bot]

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.

[Early Warning System Bot]

Comment on attachment 119112 [details]
Patch

Attachment 119112 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10876038

[Gyuyoung Kim]

Comment on attachment 119112 [details]
Patch

Attachment 119112 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10876045

[Gustavo Noronha (kov)]

Comment on attachment 119112 [details]
Patch

Attachment 119112 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10873050

[Collabora GTK+ EWS bot]

Comment on attachment 119112 [details]
Patch

Attachment 119112 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10875113

[Mark Hahnenberg]

Created attachment 119263 [details]
Patch

[WebKit Review Bot]

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.

[Geoffrey Garen]

Comment on attachment 119263 [details]
Patch

Since I've already reviewed the sub-patches, r=me if it builds.

[Early Warning System Bot]

Comment on attachment 119263 [details]
Patch

Attachment 119263 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10872346

[Gyuyoung Kim]

Comment on attachment 119263 [details]
Patch

Attachment 119263 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10872357

[Gustavo Noronha (kov)]

Comment on attachment 119263 [details]
Patch

Attachment 119263 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10873360

[Gustavo Noronha (kov)]

Comment on attachment 119263 [details]
Patch

Attachment 119263 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10870456

[Mark Hahnenberg]

Created attachment 119317 [details]
Patch

[WebKit Review Bot]

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.

[Gyuyoung Kim]

Comment on attachment 119317 [details]
Patch

Attachment 119317 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10905021

[Mark Hahnenberg]

Created attachment 119371 [details]
Patch

[WebKit Review Bot]

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.

[Geoffrey Garen]

Comment on attachment 119371 [details]
Patch

r=me if it builds.

[WebKit Review Bot]

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

[Mark Hahnenberg]

Committed r103083: <http://trac.webkit.org/changeset/103083>

[Mark Hahnenberg]

Created attachment 119639 [details]
Bencher results

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