We currently do a really bizarre dance when creating code blocks. When compiling, we typically install the compiled code in Executable first and then in the CodeBlock - that's messed up. We establish the Executable<->CodeBlock relationship at random times depending on code path. We have duplicated code between the LLInt initialization path, the baseline JIT compilation path, and the DFG/FTL compilation paths. It's incredibly confusing. This confusion makes it hard to do things like creating a custom temporary CodeBlock just for DFG->FTL OSR entry, or having the DFG trigger its own replacement compilation (i.e. DFG->FTL tier-up). It also means that the DFG::Plan and DFG::Worklist have super-special knowledge of the CodeBlock<->Executable relationship. That makes it hard to establish new such relationships, for example for FTL code. Instead we should have a simplified system of rules: 1) You can always ask an Executable for a new code block. It will give you one, and you will own it for now. You can hold onto it for as long as you like. 2) You can always ask an Executable to install a CodeBlock. It will do all of the things needed to ensure that calls are redirected from the old one to the new one, and that the GC knows about the increased costs. 3) You can always ask a CodeBlock to prepare itself for execution - i.e. go from being just a thing that holds bytecode to being a thing that has proper entrypoints. When you make such a request, you should be able to specify which execution engine to use, and the CodeBlock should figure it out. 4) Asking a CodeBlock to prepare for execution should have an optional asynchronous mode, in which case you should pass a callback describing what to do when compilation finishes.
Created attachment 209681 [details] work in progress
Created attachment 209794 [details] almost there
Created attachment 209797 [details] more
Created attachment 209834 [details] almost compiles
Created attachment 209837 [details] almost Almost done. I think I just need to write some FTL glue to keep FTL builds working and then I'll be done.
Attachment 209837 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/bytecode/CodeBlock.cpp', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/bytecode/DeferredCompilationCallback.cpp', u'Source/JavaScriptCore/bytecode/DeferredCompilationCallback.h', u'Source/JavaScriptCore/dfg/DFGDriver.cpp', u'Source/JavaScriptCore/dfg/DFGDriver.h', u'Source/JavaScriptCore/dfg/DFGFailedFinalizer.cpp', u'Source/JavaScriptCore/dfg/DFGFailedFinalizer.h', u'Source/JavaScriptCore/dfg/DFGFinalizer.h', u'Source/JavaScriptCore/dfg/DFGJITFinalizer.cpp', u'Source/JavaScriptCore/dfg/DFGJITFinalizer.h', u'Source/JavaScriptCore/dfg/DFGOSRExitPreparation.cpp', u'Source/JavaScriptCore/dfg/DFGOperations.cpp', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/dfg/DFGPlan.h', u'Source/JavaScriptCore/dfg/DFGWorklist.cpp', u'Source/JavaScriptCore/heap/Heap.h', u'Source/JavaScriptCore/interpreter/Interpreter.cpp', u'Source/JavaScriptCore/jit/JITDriver.h', u'Source/JavaScriptCore/jit/JITStubs.cpp', u'Source/JavaScriptCore/jit/JITToDFGDeferredCompilationCallback.cpp', u'Source/JavaScriptCore/jit/JITToDFGDeferredCompilationCallback.h', u'Source/JavaScriptCore/llint/LLIntEntrypoints.cpp', u'Source/JavaScriptCore/llint/LLIntEntrypoints.h', u'Source/JavaScriptCore/llint/LLIntSlowPaths.cpp', u'Source/JavaScriptCore/runtime/ArrayPrototype.cpp', u'Source/JavaScriptCore/runtime/CommonSlowPaths.cpp', u'Source/JavaScriptCore/runtime/CompilationResult.cpp', u'Source/JavaScriptCore/runtime/CompilationResult.h', u'Source/JavaScriptCore/runtime/Executable.cpp', u'Source/JavaScriptCore/runtime/Executable.h', u'Source/JavaScriptCore/runtime/ExecutionHarness.h']" exit_code: 1 Source/JavaScriptCore/jit/JITStubs.cpp:1058: The return type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/JavaScriptCore/dfg/DFGPlan.h:56: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 30 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #6) > Attachment 209837 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/bytecode/CodeBlock.cpp', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/bytecode/DeferredCompilationCallback.cpp', u'Source/JavaScriptCore/bytecode/DeferredCompilationCallback.h', u'Source/JavaScriptCore/dfg/DFGDriver.cpp', u'Source/JavaScriptCore/dfg/DFGDriver.h', u'Source/JavaScriptCore/dfg/DFGFailedFinalizer.cpp', u'Source/JavaScriptCore/dfg/DFGFailedFinalizer.h', u'Source/JavaScriptCore/dfg/DFGFinalizer.h', u'Source/JavaScriptCore/dfg/DFGJITFinalizer.cpp', u'Source/JavaScriptCore/dfg/DFGJITFinalizer.h', u'Source/JavaScriptCore/dfg/DFGOSRExitPreparation.cpp', u'Source/JavaScriptCore/dfg/DFGOperations.cpp', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/dfg/DFGPlan.h', u'Source/JavaScriptCore/dfg/DFGWorklist.cpp', u'Source/JavaScriptCore/heap/Heap.h', u'Source/JavaScriptCore/interpreter/Interpreter.cpp', u'Source/JavaScriptCore/jit/JITDriver.h', u'Source/JavaScriptCore/jit/JITStubs.cpp', u'Source/JavaScriptCore/jit/JITToDFGDeferredCompilationCallback.cpp', u'Source/JavaScriptCore/jit/JITToDFGDeferredCompilationCallback.h', u'Source/JavaScriptCore/llint/LLIntEntrypoints.cpp', u'Source/JavaScriptCore/llint/LLIntEntrypoints.h', u'Source/JavaScriptCore/llint/LLIntSlowPaths.cpp', u'Source/JavaScriptCore/runtime/ArrayPrototype.cpp', u'Source/JavaScriptCore/runtime/CommonSlowPaths.cpp', u'Source/JavaScriptCore/runtime/CompilationResult.cpp', u'Source/JavaScriptCore/runtime/CompilationResult.h', u'Source/JavaScriptCore/runtime/Executable.cpp', u'Source/JavaScriptCore/runtime/Executable.h', u'Source/JavaScriptCore/runtime/ExecutionHarness.h']" exit_code: 1 > Source/JavaScriptCore/jit/JITStubs.cpp:1058: The return type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] False. > Source/JavaScriptCore/dfg/DFGPlan.h:56: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Fixed. > Total errors found: 2 in 30 files > > > If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 209837 [details] almost Attachment 209837 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1605112
Comment on attachment 209837 [details] almost Attachment 209837 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/1625122
Comment on attachment 209837 [details] almost Attachment 209837 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1619121
Comment on attachment 209837 [details] almost Attachment 209837 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1628129
Comment on attachment 209837 [details] almost Attachment 209837 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/1626109
Comment on attachment 209837 [details] almost Attachment 209837 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/1623130
Created attachment 209845 [details] the patch
Attachment 209845 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/Target.pri', u'Source/JavaScriptCore/bytecode/CodeBlock.cpp', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/bytecode/DeferredCompilationCallback.cpp', u'Source/JavaScriptCore/bytecode/DeferredCompilationCallback.h', u'Source/JavaScriptCore/dfg/DFGDriver.cpp', u'Source/JavaScriptCore/dfg/DFGDriver.h', u'Source/JavaScriptCore/dfg/DFGFailedFinalizer.cpp', u'Source/JavaScriptCore/dfg/DFGFailedFinalizer.h', u'Source/JavaScriptCore/dfg/DFGFinalizer.h', u'Source/JavaScriptCore/dfg/DFGJITFinalizer.cpp', u'Source/JavaScriptCore/dfg/DFGJITFinalizer.h', u'Source/JavaScriptCore/dfg/DFGOSRExitPreparation.cpp', u'Source/JavaScriptCore/dfg/DFGOperations.cpp', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/dfg/DFGPlan.h', u'Source/JavaScriptCore/dfg/DFGWorklist.cpp', u'Source/JavaScriptCore/heap/Heap.h', u'Source/JavaScriptCore/interpreter/Interpreter.cpp', u'Source/JavaScriptCore/jit/JITDriver.h', u'Source/JavaScriptCore/jit/JITStubs.cpp', u'Source/JavaScriptCore/jit/JITToDFGDeferredCompilationCallback.cpp', u'Source/JavaScriptCore/jit/JITToDFGDeferredCompilationCallback.h', u'Source/JavaScriptCore/llint/LLIntEntrypoints.cpp', u'Source/JavaScriptCore/llint/LLIntEntrypoints.h', u'Source/JavaScriptCore/llint/LLIntSlowPaths.cpp', u'Source/JavaScriptCore/runtime/ArrayPrototype.cpp', u'Source/JavaScriptCore/runtime/CommonSlowPaths.cpp', u'Source/JavaScriptCore/runtime/CompilationResult.cpp', u'Source/JavaScriptCore/runtime/CompilationResult.h', u'Source/JavaScriptCore/runtime/Executable.cpp', u'Source/JavaScriptCore/runtime/Executable.h', u'Source/JavaScriptCore/runtime/ExecutionHarness.h']" exit_code: 1 Source/JavaScriptCore/jit/JITStubs.cpp:1058: The return type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 1 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 209846 [details] the patch
Attachment 209846 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/GNUmakefile.list.am', u'Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/Target.pri', u'Source/JavaScriptCore/bytecode/CodeBlock.cpp', u'Source/JavaScriptCore/bytecode/CodeBlock.h', u'Source/JavaScriptCore/bytecode/DeferredCompilationCallback.cpp', u'Source/JavaScriptCore/bytecode/DeferredCompilationCallback.h', u'Source/JavaScriptCore/dfg/DFGDriver.cpp', u'Source/JavaScriptCore/dfg/DFGDriver.h', u'Source/JavaScriptCore/dfg/DFGFailedFinalizer.cpp', u'Source/JavaScriptCore/dfg/DFGFailedFinalizer.h', u'Source/JavaScriptCore/dfg/DFGFinalizer.h', u'Source/JavaScriptCore/dfg/DFGJITFinalizer.cpp', u'Source/JavaScriptCore/dfg/DFGJITFinalizer.h', u'Source/JavaScriptCore/dfg/DFGOSRExitPreparation.cpp', u'Source/JavaScriptCore/dfg/DFGOperations.cpp', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/dfg/DFGPlan.h', u'Source/JavaScriptCore/dfg/DFGWorklist.cpp', u'Source/JavaScriptCore/heap/Heap.h', u'Source/JavaScriptCore/interpreter/Interpreter.cpp', u'Source/JavaScriptCore/jit/JITDriver.h', u'Source/JavaScriptCore/jit/JITStubs.cpp', u'Source/JavaScriptCore/jit/JITToDFGDeferredCompilationCallback.cpp', u'Source/JavaScriptCore/jit/JITToDFGDeferredCompilationCallback.h', u'Source/JavaScriptCore/llint/LLIntEntrypoints.cpp', u'Source/JavaScriptCore/llint/LLIntEntrypoints.h', u'Source/JavaScriptCore/llint/LLIntSlowPaths.cpp', u'Source/JavaScriptCore/runtime/ArrayPrototype.cpp', u'Source/JavaScriptCore/runtime/CommonSlowPaths.cpp', u'Source/JavaScriptCore/runtime/CompilationResult.cpp', u'Source/JavaScriptCore/runtime/CompilationResult.h', u'Source/JavaScriptCore/runtime/Executable.cpp', u'Source/JavaScriptCore/runtime/Executable.h', u'Source/JavaScriptCore/runtime/ExecutionHarness.h']" exit_code: 1 Source/JavaScriptCore/jit/JITStubs.cpp:1058: The return type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 1 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 209846 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=209846&action=review r=me if you switch to having create methods for the new refcounted classes > Source/JavaScriptCore/bytecode/DeferredCompilationCallback.h:38 > +public: > + DeferredCompilationCallback(); Can we give this a protected constructor and a public create method? > Source/JavaScriptCore/jit/JITToDFGDeferredCompilationCallback.h:37 > +public: > + JITToDFGDeferredCompilationCallback(); can we make the constructor private, and give this a PassRefPtr returning create() method?
(In reply to comment #18) > (From update of attachment 209846 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=209846&action=review > > r=me if you switch to having create methods for the new refcounted classes > > > Source/JavaScriptCore/bytecode/DeferredCompilationCallback.h:38 > > +public: > > + DeferredCompilationCallback(); > > Can we give this a protected constructor and a public create method? Protected constructor yes - but no need for a create method since it's an abstract base class. > > > Source/JavaScriptCore/jit/JITToDFGDeferredCompilationCallback.h:37 > > +public: > > + JITToDFGDeferredCompilationCallback(); > > can we make the constructor private, and give this a PassRefPtr returning create() method? Fixed.
Landed in http://trac.webkit.org/changeset/154804
Re-opened since this is blocked by bug 120477
Relanded in http://trac.webkit.org/changeset/154824