Bug 74331

Summary:

De-virtualize destructors

Product:

WebKit

Reporter:

Mark Hahnenberg <mhahnenberg>

Component:

JavaScriptCore

Assignee:

Mark Hahnenberg <mhahnenberg>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, gustavo.noronha, gustavo, japhet, webkit.review.bot, xan.lopez

Priority:

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

Mark Hahnenberg

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

Attachments
Patch (278.16 KB, patch) 2011-12-13 15:02 PST, Mark Hahnenberg	no flags	Details Formatted Diff Diff
Patch (282.01 KB, patch) 2011-12-13 17:02 PST, Mark Hahnenberg	no flags	Details Formatted Diff Diff
Patch (302.79 KB, patch) 2011-12-14 11:28 PST, Mark Hahnenberg	no flags	Details Formatted Diff Diff
Patch (303.14 KB, patch) 2011-12-14 16:11 PST, Mark Hahnenberg	no flags	Details Formatted Diff Diff
Patch (302.96 KB, patch) 2011-12-14 21:27 PST, Mark Hahnenberg	ggaren: review+ webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Bencher results (7.03 KB, text/plain) 2011-12-16 11:22 PST, Mark Hahnenberg	no flags	Details
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Mark Hahnenberg

Comment 1 2011-12-13 15:02:46 PST

Created attachment 119091 [details] Patch

Mark Hahnenberg

Comment 2 2011-12-13 17:02:17 PST

Created attachment 119112 [details] Patch

Mark Hahnenberg

Comment 3 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.

WebKit Review Bot

Comment 4 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.

Early Warning System Bot

Comment 5 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

Gyuyoung Kim

Comment 6 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

Gustavo Noronha (kov)

Comment 7 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

Collabora GTK+ EWS bot

Comment 8 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

Mark Hahnenberg

Comment 9 2011-12-14 11:28:56 PST

Created attachment 119263 [details] Patch

WebKit Review Bot

Comment 10 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.

Geoffrey Garen

Comment 11 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.

Early Warning System Bot

Comment 12 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

Gyuyoung Kim

Comment 13 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

Gustavo Noronha (kov)

Comment 14 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

Gustavo Noronha (kov)

Comment 15 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

Mark Hahnenberg

Comment 16 2011-12-14 16:11:51 PST

Created attachment 119317 [details] Patch

WebKit Review Bot

Comment 17 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.

Gyuyoung Kim

Comment 18 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

Mark Hahnenberg

Comment 19 2011-12-14 21:27:56 PST

Created attachment 119371 [details] Patch

WebKit Review Bot

Comment 20 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.

Geoffrey Garen

Comment 21 2011-12-15 18:47:11 PST

Comment on attachment 119371 [details] Patch r=me if it builds.

WebKit Review Bot

Comment 22 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

Mark Hahnenberg

Comment 23 2011-12-16 11:13:08 PST

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

Mark Hahnenberg

Comment 24 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.

Note You need to log in before you can comment on or make changes to this bug.