When tiered compilation decides to optimize a code block, some value profiles might not contain sufficient information to make wise optimization decisions. Tiered compilation does not take this into account. One way that tiered compilation could take this into account is to postpone optimization a bit if some value profiles are unusually empty. It can also work around problems with value profiles containing only a few samples due to resonance in the bucketCounterRegister by converting the buckets into a prediction and then clearing them, so that even with resonance happening, after a few iterations of optimization postponement the prediction will contain everything we need to know.
Created attachment 107395 [details] the patch
Attachment 107395 [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/ValueProfile.h:278: The parameter name "classInfo" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/ValueProfile.h:278: The parameter name "statistics" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/bytecode/ValueProfile.h:281: The parameter name "statistics" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 3 in 18 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 107398 [details] the patch - fix style
Comment on attachment 107395 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=107395&action=review Basically okay, but it needs an actual changelog and can you convert the magic numbers into named constants? Given you're adding a new cpp file you'll need to add it to GNUMakefile, JavaScriptCore.pro and CMakeLists.txt > Source/JavaScriptCore/ChangeLog:7 > + You do need an actual changelog entry you know :D > Source/JavaScriptCore/jit/JIT.cpp:102 > - Jump skipOptimize = branchAdd32(Signed, TrustedImm32(kind == LoopOptimizationCheck ? 1 : 30), AbsoluteAddress(&m_codeBlock->m_executeCounter)); > + Jump skipOptimize = branchAdd32(Signed, TrustedImm32(kind == LoopOptimizationCheck ? 1 : 15), AbsoluteAddress(m_codeBlock->addressOfExecuteCounter())); Ahhh magic numbers!!! L-( I have no idea what this number means -- can we hoist it into a named constant?
Created attachment 107399 [details] the patch
Comment on attachment 107399 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=107399&action=review Correct the printf bit before landing. Would be nice to avoid unnecessary renames (buckets->m_buckets, etc) in future, as they add noise to the patch. > Source/JavaScriptCore/jit/JITStubs.cpp:1840 > + if (error) > + fprintf(stderr, "WARNING: optimized compilation from loop failed.\n"); Should be in a VERBOSE_OSR block? Future patch might want to define a OSR_LOG() macro which might be nicer in the long run
Comment on attachment 107399 [details] the patch Attachment 107399 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9661650
Comment on attachment 107399 [details] the patch Attachment 107399 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/9659685
Landed in r95134.