Bug 128072

Created attachment 222934 [details] the patch

Attachment 222934 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/bytecode/CodeBlock.cpp:354: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/bytecode/CodeBlock.cpp:414: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/bytecode/CodeBlock.cpp:419: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/bytecode/CodeBlock.cpp:424: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 4 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 222935 [details] the patch

Comment on attachment 222935 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=222935&action=review > Source/JavaScriptCore/bytecode/CodeBlock.cpp:2059 > +// Returns false if we want this reconsidered. > +static void forceStructureLiveness( I think this comment got out of date while you were writing the patch.

(In reply to comment #4) > (From update of attachment 222935 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=222935&action=review > > > Source/JavaScriptCore/bytecode/CodeBlock.cpp:2059 > > +// Returns false if we want this reconsidered. > > +static void forceStructureLiveness( > > I think this comment got out of date while you were writing the patch. Yup, removed.

Further testing shows that this is basically blocked on https://bugs.webkit.org/show_bug.cgi?id=128073, and perhaps something even deeper than that: we need to be able to add the ability to: - Track larger StructureSets for prorotype/chain access inlining. - Convert GetByIds to inlined prototype/chain accesses. Also, I think that this experience is suggesting that our baseline PIC's are going polymorphic too eagerly. Perhaps a good heuristic would be to let them ping-pong N times before going polymorphic.

Created attachment 223213 [details] another version I no longer think that this is a particularly good idea.

Comment on attachment 222935 [details] the patch Cleared Oliver Hunt's review+ from obsolete attachment 222935 [details] so that this bug does not appear in http://webkit.org/pending-commit.

Created attachment 233923 [details] work in progress This uses a slightly different approach. It's almost done.

Created attachment 233951 [details] the patch

Comment on attachment 233951 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=233951&action=review r=me, with some fixate > Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:85 > + if (false && jit.codeBlock()->instructionCount() == 681) > + jit.breakpoint(); Please remove. > Source/JavaScriptCore/runtime/Executable.h:457 > + bool m_didTryToEnterInLoop; It looks like this data member is not initialized in the constructor.

Nooope. Testing shows that actually, even with all of the work I did, the GC clearing structures is still profitable. That said, we're now in a situation where it is less profitable on gbemu (i.e. it's not absolutely necessary for performance) and less evil (it's not necessary to assume that if the GC clears IC's then the IC's need to be forever polymorphic).

Comment on attachment 233951 [details] the patch Cleared Geoffrey Garen's review+ from obsolete attachment 233951 [details] so that this bug does not appear in http://webkit.org/pending-commit.

Oh we did something much stronger than this. *** This bug has been marked as a duplicate of bug 157324 ***