Patch forthcoming.
Created attachment 178289 [details] the patch
Attachment 178289 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/dfg/DFGCapabilities.h:42: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGCapabilities.h:43: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGCapabilities.h:44: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGCapabilities.h:45: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGCapabilities.h:46: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGCapabilities.h:47: The parameter name "codeBlock" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 6 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 178289 [details] the patch Attachment 178289 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15173983
Windows export file needs some babysitting: 6>CodeBlock.obj : error LNK2019: unresolved external symbol "public: int __thiscall JSC::CodeBlock::counterValueForOptimizeAfterWarmUp(void)" (?counterValueForOptimizeAfterWarmUp@CodeBlock@JSC@@QAEHXZ) referenced in function "public: void __thiscall JSC::CodeBlock::optimizeAfterWarmUp(void)" (?optimizeAfterWarmUp@CodeBlock@JSC@@QAEXXZ)
Comment on attachment 178289 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=178289&action=review r=me, please fix comments before landing. > Source/JavaScriptCore/dfg/DFGCapabilities.h:48 > +bool mightCompileEval(CodeBlock* codeBlock); > +bool mightCompileProgram(CodeBlock* codeBlock); > +bool mightCompileFunctionForCall(CodeBlock* codeBlock); > +bool mightCompileFunctionForConstruct(CodeBlock* codeBlock); > +bool mightInlineFunctionForCall(CodeBlock* codeBlock); > +bool mightInlineFunctionForConstruct(CodeBlock* codeBlock); > Style bot is right here, for once.
(In reply to comment #4) > Windows export file needs some babysitting: > > > 6>CodeBlock.obj : error LNK2019: unresolved external symbol "public: int __thiscall JSC::CodeBlock::counterValueForOptimizeAfterWarmUp(void)" (?counterValueForOptimizeAfterWarmUp@CodeBlock@JSC@@QAEHXZ) referenced in function "public: void __thiscall JSC::CodeBlock::optimizeAfterWarmUp(void)" (?optimizeAfterWarmUp@CodeBlock@JSC@@QAEXXZ) I think it's once again misuse of #ifdef's like ENABLE(DFG.. and so on. :-/ I'm trying to fiddle with it now.
(In reply to comment #5) > (From update of attachment 178289 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=178289&action=review > > r=me, please fix comments before landing. > > > Source/JavaScriptCore/dfg/DFGCapabilities.h:48 > > +bool mightCompileEval(CodeBlock* codeBlock); > > +bool mightCompileProgram(CodeBlock* codeBlock); > > +bool mightCompileFunctionForCall(CodeBlock* codeBlock); > > +bool mightCompileFunctionForConstruct(CodeBlock* codeBlock); > > +bool mightInlineFunctionForCall(CodeBlock* codeBlock); > > +bool mightInlineFunctionForConstruct(CodeBlock* codeBlock); > > > > Style bot is right here, for once. Yup, I fixed.
Created attachment 178303 [details] work in progress This should make EWS happy.
Comment on attachment 178303 [details] work in progress This causes a massive regression on Kraken audio benchmarks. The regression only shows up in browser. Investigating the reasons now.
It looks like what is going on is that we've got some code blocks in the Kraken audio benchmarks that rely heavily on OSR exit profiling. Our previous threshold for triggering recompilation was 5000 (thresholdForOptimizeAfterLongWarmUp); this code effectively increases that threshold since we have scaling by instruction count and most code blocks that benefit from recompilation tend to be large-ish. So, I experimented with lowering the thresholdForOptimizeAfterLongWarmUp. With 3000, we already neutralize the slow-down on Kraken. But lowering it further seems to help even more. It looks like setting it to 1000 helps the most. Here are the performance numbers on various benchmarks for 1000, 2000, and 3000: 3000: jsc: 0.7% faster on SunSpider neutral on V8v7 ~1% slower on Kraken drt: 1% faster on SunSpider ~1% faster on V8v7 neutral on Octane 2% slower on Kraken ~10% faster on JSBench 1.5% faster on DSP 2000: jsc: 0.9% faster on SunSpider neutral on V8v7 0.9% slower on Kraken drt: neutral on SunSpider 0.6% faster on V8v7 ~2% slower on Kraken ~7.8% faster on JSBench 1.9% faster on DSP 1000: jsc: 1.4% faster on SunSpider 0.4% slower on V8v7 0.6% slower on Kraken drt: 0.6% faster on SunSpider 1.3% faster on V8v7 neutral on Kraken 1% faster on JSBench neutral on DSP Further tests seem to imply that the speed-up on SunSpider in DRT is closer to 1% with 1000, but I can't be sure. Anyway, 1000 seems best since I'd rather be neutral on Kraken and get a 1% V8v7 & SunSpider speed-up.
Created attachment 178396 [details] the patch I fixed all regressions. It took some effort.
Attachment 178396 [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/bytecode/CodeBlock.cpp:3110: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/bytecode/CodeBlock.cpp:3115: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/JavaScriptCore/bytecode/CodeBlock.cpp:3125: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 3 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 178397 [details] the patch fix style
Comment on attachment 178397 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=178397&action=review r=me > Source/JavaScriptCore/profiler/ProfilerDatabase.cpp:61 > + Bytecodes* result = addBytecodes( > + codeBlock->hash(), codeBlock->sourceCodeForTools()); something this short could just be a single line :p
(In reply to comment #14) > (From update of attachment 178397 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=178397&action=review > > r=me > > > Source/JavaScriptCore/profiler/ProfilerDatabase.cpp:61 > > + Bytecodes* result = addBytecodes( > > + codeBlock->hash(), codeBlock->sourceCodeForTools()); > > something this short could just be a single line :p You make a fair point! Fixed.
Landed in http://trac.webkit.org/changeset/137094
Should we reconsider these limits, now that bug 104500 has been discovered and fixed?