JetStream2/UniPoker uses String#codePointAt much. Sampling rate: 1000.000000 microseconds Top functions as <numSamples 'functionName:sourceID'> 285 '[Symbol.match]:3' 176 'shift:-1' 140 'codePointAt:-1' 111 'join:-1' 77 'shuffle:4' 77 'merge:3' 67 'sort:3' 50 'mergeSort:3' 44 'score:4' 40 'arrayIteratorValueNext:3' 35 'cardRank:4' 35 ':5' Sampling rate: 1000.000000 microseconds Hottest bytecodes as <numSamples 'functionName#hash:JITType:bytecodeIndex'> 183 '[Symbol.match]#B7PWYa:Baseline:82 <-- score#Duc7t2:FTL:163' 176 'shift#<nil>:None:<nil>' 140 'codePointAt#<nil>:None:<nil>' 111 'join#<nil>:None:<nil>' 47 '[Symbol.match]#B7PWYa:Baseline:82 <-- score#Duc7t2:FTL:127' 41 'shuffle#A4Tfzo:FTL:100' 30 '[Symbol.match]#B7PWYa:Baseline:82 <-- score#Duc7t2:FTL:91' 23 'mergeSort#DOiQtk:FTL:22' 21 'indexOf#<nil>:None:<nil>' 20 '[Symbol.match]#B7PWYa:Baseline:82 <-- score#Duc7t2:FTL:715' 18 'clear#CPW4SA:Baseline:13 <-- playHands#DltzVB:FTL:111' 17 'sort#C0wzoE:FTL:10' 14 'mergeSort#DOiQtk:FTL:96' 14 'cardRank#B8GTs9:Baseline:21 <-- merge#AVbsbl:FTL:151' 12 'merge#AVbsbl:FTL:0' 11 'merge#AVbsbl:FTL:185' 11 'shuffle#A4Tfzo:FTL:113' 10 'newDeck#B6KIhS:Baseline:43 <-- shuffle#A4Tfzo:FTL:18' 9 'shuffle#A4Tfzo:FTL:88' 8 'cardRank#B8GTs9:Baseline:13 <-- merge#AVbsbl:FTL:151' 8 'sort#C0wzoE:FTL:114' 8 'score#Duc7t2:FTL:682' 8 'sort#C0wzoE:FTL:66' 7 'merge#AVbsbl:FTL:212' 6 '#EMWXIv:FTL:0' 6 'score#Duc7t2:FTL:1555' 6 'merge#AVbsbl:FTL:95' 6 'score#Duc7t2:FTL:32' 5 'merge#AVbsbl:FTL:224' 5 'sort#C0wzoE:FTL:126' 5 '#EMWXIv:FTL:296' 5 'merge#AVbsbl:FTL:151' 5 'sort#C0wzoE:FTL:42' 5 'mergeSort#DOiQtk:FTL:16' 5 'arrayIteratorValueNext#ANxshv:Baseline:16 <-- score#Duc7t2:FTL:2068' 5 'merge#AVbsbl:FTL:46' 5 'merge#AVbsbl:FTL:166' 5 'sort#C0wzoE:FTL:78' 4 'sort#C0wzoE:FTL:54' 4 'shuffle#A4Tfzo:FTL:131'
<rdar://problem/55246768>
Ensured that this offers more than 10% Score improvement in JetStream2/UniPoker.
Created attachment 378538 [details] Patch
Created attachment 378539 [details] Patch
Created attachment 378546 [details] Patch
Comment on attachment 378546 [details] Patch Attachment 378546 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/13022548 New failing tests: stress/string-char-code-at-constant-index-out-of-range.js.no-llint stress/string-char-code-at-constant-index-out-of-range.js.ftl-no-cjit-validate-sampling-profiler stress/string-char-code-at-constant-index-out-of-range.js.bytecode-cache stress/string-char-code-at-constant-index-out-of-range.js.ftl-no-cjit-b3o0 stress/string-char-code-at-constant-index-out-of-range.js.no-cjit-validate-phases stress/string-char-code-at-constant-index-out-of-range.js.ftl-no-cjit-small-pool stress/string-char-code-at-constant-index-out-of-range.js.dfg-eager-no-cjit-validate stress/string-char-code-at-constant-index-out-of-range.js.ftl-no-cjit-no-put-stack-validate stress/string-char-code-at-constant-index-out-of-range.js.ftl-no-cjit-no-inline-validate stress/string-char-code-at-constant-index-out-of-range.js.no-cjit-collect-continuously stress/string-char-code-at-constant-index-out-of-range.js.ftl-eager-no-cjit stress/string-char-code-at-constant-index-out-of-range.js.no-ftl stress/string-char-code-at-constant-index-out-of-range.js.ftl-eager-no-cjit-b3o1 stress/string-char-code-at-constant-index-out-of-range.js.default stress/string-char-code-at-constant-index-out-of-range.js.dfg-eager stress/string-char-code-at-constant-index-out-of-range.js.eager-jettison-no-cjit stress/string-char-code-at-constant-index-out-of-range.js.mini-mode stress/string-char-code-at-constant-index-out-of-range.js.ftl-eager
Comment on attachment 378546 [details] Patch New test stress/string-char-code-at-constant-index-out-of-range.js fails.
(In reply to Michael Saboff from comment #7) > Comment on attachment 378546 [details] > Patch > > New test stress/string-char-code-at-constant-index-out-of-range.js fails. Oops, the test is wrong, fixing.
Created attachment 378565 [details] Patch
Comment on attachment 378565 [details] Patch r=me
Committed r249780: <https://trac.webkit.org/changeset/249780>
Comment on attachment 378565 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378565&action=review LGTM too. A couple questions > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2629 > + if (argumentCountIncludingThis != 2) Why not “< 2”? > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2632 > + if (m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, Uncountable)) CheckArray is Uncountable?
Comment on attachment 378565 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378565&action=review Nice >> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2629 >> + if (argumentCountIncludingThis != 2) > > Why not “< 2”? Doing `< 2` sounds nice. I'll change this for the other DFG nodes (StringCharAt / StringCharCodeAt). >> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2632 >> + if (m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, Uncountable)) > > CheckArray is Uncountable? No, CheckArray emits other thing. Uncountable is checking negative indexes in StringCodePointAt. We should have exit handing for CheckArray here. And I think StringCharCodeAt and StringCharAt are also missing CheckArray exits. Need to add them.
Comment on attachment 378565 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378565&action=review >>> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2629 >>> + if (argumentCountIncludingThis != 2) >> >> Why not “< 2”? > > Doing `< 2` sounds nice. I'll change this for the other DFG nodes (StringCharAt / StringCharCodeAt). Yeah I've noticed this pattern in our intrinsic inlining where we check for exact argument count match (or a few potential values), instead of just checking a range. I think checking a range is more complete. (Obviously, it's unlikely these are called with more parameters, but we shouldn't disallow it). When I inline intrinsics, I've been trying to use the "< X" paradigm. >>> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2632 >>> + if (m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, Uncountable)) >> >> CheckArray is Uncountable? > > No, CheckArray emits other thing. Uncountable is checking negative indexes in StringCodePointAt. We should have exit handing for CheckArray here. > And I think StringCharCodeAt and StringCharAt are also missing CheckArray exits. Need to add them. 👍🏼