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: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, fpizlo, ggaren, mhahnenberg, msaboff, oliver, ossy, paroga, rakuco, webkit.review.bot
Priority: P2    
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+

Description Filip Pizlo 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.
Comment 1 Filip Pizlo 2012-07-07 14:42:11 PDT 
Created attachment 151147 [details]
work in progress

Oh boy, another huge patch.
Comment 2 Filip Pizlo 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.
Comment 3 Filip Pizlo 2012-07-08 19:04:47 PDT 
Created attachment 151169 [details]
the patch

It seems to work.
Comment 4 WebKit Review Bot 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.
Comment 5 Early Warning System Bot 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
Comment 6 Build Bot 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
Comment 7 Early Warning System Bot 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
Comment 8 Gyuyoung Kim 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
Comment 9 Filip Pizlo 2012-07-08 19:49:11 PDT 
Created attachment 151171 [details]
the patch

Got it to, like, build, and stuff.
Comment 10 WebKit Review Bot 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.
Comment 11 Filip Pizlo 2012-07-08 19:57:40 PDT 
Created attachment 151172 [details]
the patch

Fixed some of the style errors.
Comment 12 WebKit Review Bot 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.
Comment 13 Filip Pizlo 2012-07-08 20:06:43 PDT 
Created attachment 151173 [details]
the patch
Comment 14 WebKit Review Bot 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.
Comment 15 Early Warning System Bot 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
Comment 16 Early Warning System Bot 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
Comment 17 Filip Pizlo 2012-07-08 20:46:54 PDT 
Created attachment 151175 [details]
the patch

More random fixes.
Comment 18 WebKit Review Bot 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.
Comment 19 Filip Pizlo 2012-07-08 22:16:57 PDT 
Created attachment 151185 [details]
the patch

Fixed the last bugs.  It passes tests and is perf neutral.
Comment 20 WebKit Review Bot 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.
Comment 21 Gavin Barraclough 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.
Comment 22 Filip Pizlo 2012-07-09 16:27:40 PDT 
Landed in http://trac.webkit.org/changeset/122166
Comment 23 Csaba Osztrogonác 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.
Comment 25 Patrick R. Gansterer 2012-07-19 01:05:54 PDT 
PING? !ENABLE(JIT) still does not build.
Comment 26 Filip Pizlo 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