Bug 90731

Summary:

It should be possible to jettison JIT stub routines even if they are currently running

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, fpizlo, ggaren, mhahnenberg, msaboff, oliver, ossy, paroga, rakuco, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

90852

Bug Blocks:

Attachments:

Description	Flags
work in progress	none
wrote a tiny bit more code	none
the patch	webkit-ews: commit-queue-
the patch	none
the patch	none
the patch	webkit-ews: commit-queue-
the patch	none
the patch	barraclough: review+

Filip Pizlo

Reported 2012-07-07 14:34:49 PDT

Currently it's not possible to delete JIT stub routines except during garbage collection. Even then, this barely works because the JIT stub routine that we want to kill may be currently on the stack. Right now we make it work by ensuring that (a) only the GC can decide to kill stub routines and (b) it will only kill them if the pointers they rely on are dead. But it's not clear that this is quite correct, since we may choose to kill a list of stubs if any stub in the list has stale pointers. So if a stub is currently running, and its pointers are live because of some other black magic we pulled, then it may still get killed by GC because it belongs to a list of stubs where some other stub is proved dead. This needs to all be rationalized: 1) We don't want to be killing stubs prematurely. That's probably bad. 2) We want to be able to kill stubs even if they are running. The best way to do this is to jettison-to-GC like we do for reoptimization: we can mark the stub routine as having liveness that is predicated on it being on the stack *right now*. As soon as the GC proves that it is not on the stack, it can kill it. Work in progress patch forthcoming.

Attachments
work in progress (63.52 KB, patch) 2012-07-07 14:42 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
wrote a tiny bit more code (80.42 KB, patch) 2012-07-07 15:12 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (99.54 KB, patch) 2012-07-08 19:04 PDT, Filip Pizlo	webkit-ews: commit-queue-	Details Formatted Diff Diff
the patch (99.64 KB, patch) 2012-07-08 19:49 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (99.62 KB, patch) 2012-07-08 19:57 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (100.00 KB, patch) 2012-07-08 20:06 PDT, Filip Pizlo	webkit-ews: commit-queue-	Details Formatted Diff Diff
the patch (100.08 KB, patch) 2012-07-08 20:46 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (100.97 KB, patch) 2012-07-08 22:16 PDT, Filip Pizlo	barraclough: review+	Details Formatted Diff Diff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2012-07-07 14:42:11 PDT

Created attachment 151147 [details] work in progress Oh boy, another huge patch.

Filip Pizlo

Comment 2 2012-07-07 15:12:46 PDT

Created attachment 151149 [details] wrote a tiny bit more code It compiles now, at least on 64-bit. I'll test it at some point.

Filip Pizlo

Comment 3 2012-07-08 19:04:47 PDT

Created attachment 151169 [details] the patch It seems to work.

WebKit Review Bot

Comment 4 2012-07-08 19:09:25 PDT

Attachment 151169 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:83: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:108: Use 0 or null instead of NULL (even in *comments*). [readability/null] [4] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:113: The parameter name "code" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:113: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:116: The parameter name "code" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:116: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:117: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/Instruction.h:75: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:75: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _proto is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _chain is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/jit/JITStubRoutine.h:53: More than one command on the same line [whitespace/newline] [4] Source/JavaScriptCore/heap/JITStubRoutineSet.h:44: More than one command on the same line [whitespace/newline] [4] Total errors found: 17 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.

Early Warning System Bot

Comment 5 2012-07-08 19:22:13 PDT

Comment on attachment 151169 [details] the patch Attachment 151169 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13164112

Build Bot

Comment 6 2012-07-08 19:23:43 PDT

Comment on attachment 151169 [details] the patch Attachment 151169 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13162137

Early Warning System Bot

Comment 7 2012-07-08 19:25:24 PDT

Comment on attachment 151169 [details] the patch Attachment 151169 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13170009

Gyuyoung Kim

Comment 8 2012-07-08 19:30:36 PDT

Comment on attachment 151169 [details] the patch Attachment 151169 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13160279

Filip Pizlo

Comment 9 2012-07-08 19:49:11 PDT

Created attachment 151171 [details] the patch Got it to, like, build, and stuff.

WebKit Review Bot

Comment 10 2012-07-08 19:52:06 PDT

Attachment 151171 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:83: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:108: Use 0 or null instead of NULL (even in *comments*). [readability/null] [4] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:113: The parameter name "code" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:113: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:116: The parameter name "code" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:116: The parameter name "globalData" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:117: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/Instruction.h:75: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:75: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _proto is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _chain is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/jit/JITStubRoutine.h:53: More than one command on the same line [whitespace/newline] [4] Source/JavaScriptCore/heap/JITStubRoutineSet.h:44: More than one command on the same line [whitespace/newline] [4] Total errors found: 17 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 11 2012-07-08 19:57:40 PDT

Created attachment 151172 [details] the patch Fixed some of the style errors.

WebKit Review Bot

Comment 12 2012-07-08 20:02:06 PDT

Attachment 151172 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:83: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:108: Use 0 or null instead of NULL (even in *comments*). [readability/null] [4] Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:116: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/Instruction.h:75: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:75: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _proto is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _chain is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 11 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 13 2012-07-08 20:06:43 PDT

Created attachment 151173 [details] the patch

WebKit Review Bot

Comment 14 2012-07-08 20:20:10 PDT

Attachment 151173 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:108: Use 0 or null instead of NULL (even in *comments*). [readability/null] [4] Source/JavaScriptCore/bytecode/Instruction.h:75: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:75: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _proto is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _chain is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 9 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.

Early Warning System Bot

Comment 15 2012-07-08 20:36:02 PDT

Comment on attachment 151173 [details] the patch Attachment 151173 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13163136

Early Warning System Bot

Comment 16 2012-07-08 20:36:45 PDT

Comment on attachment 151173 [details] the patch Attachment 151173 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13164127

Filip Pizlo

Comment 17 2012-07-08 20:46:54 PDT

Created attachment 151175 [details] the patch More random fixes.

WebKit Review Bot

Comment 18 2012-07-08 20:49:44 PDT

Attachment 151175 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:109: Use 0 or null instead of NULL (even in *comments*). [readability/null] [4] Source/JavaScriptCore/bytecode/Instruction.h:75: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:75: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _proto is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _chain is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 9 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 19 2012-07-08 22:16:57 PDT

Created attachment 151185 [details] the patch Fixed the last bugs. It passes tests and is perf neutral.

WebKit Review Bot

Comment 20 2012-07-08 22:20:40 PDT

Attachment 151185 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:109: Use 0 or null instead of NULL (even in *comments*). [readability/null] [4] Source/JavaScriptCore/bytecode/Instruction.h:75: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:75: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:84: _proto is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _stubRoutine is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _base is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/bytecode/Instruction.h:93: _chain is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 9 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.

Gavin Barraclough

Comment 21 2012-07-09 13:20:28 PDT

Comment on attachment 151185 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=151185&action=review > Source/JavaScriptCore/jit/GCAwareJITStubRoutine.cpp:62 > + ASSERT(!m_isJettisoned); Given the dual meaning of isJettisoned (to flag early destruction) this is pretty meaningless.

Filip Pizlo

Comment 22 2012-07-09 16:27:40 PDT

Landed in http://trac.webkit.org/changeset/122166

Csaba Osztrogonác

Comment 23 2012-07-10 00:05:58 PDT

(In reply to comment #22) > Landed in http://trac.webkit.org/changeset/122166 It made 170 tests crash on 32 bit platforms, see https://bugs.webkit.org/show_bug.cgi?id=90852 for details.

Patrick R. Gansterer

Comment 24 2012-07-10 08:32:22 PDT

Broke !ENABLE(JIT): http://build.webkit.org/builders/WinCE%20Release%20%28Build%29/builds/25753/steps/compile-webkit/logs/stdio

Patrick R. Gansterer

Comment 25 2012-07-19 01:05:54 PDT

PING? !ENABLE(JIT) still does not build.

Filip Pizlo

Comment 26 2012-07-19 13:15:19 PDT

(In reply to comment #25) > PING? !ENABLE(JIT) still does not build. See http://trac.webkit.org/changeset/123107

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