Currently debug dumps identify code blocks by their address in memory. This has loads of problems: - The relationship between code block and executable is not explicitly stated. So, if a code block gets inlined, the inlined code will refer to it by the address of its executable and you won't be able to quickly figure out which code block it was. - The relationship between code blocks corresponding to different tiers is not stated. So, if a code block gets optimized, or reoptimized, then it will have a different address. It's hard to search debug output for all of the "versions" of a code block. - If you want to search debug output on a 64-bit machine for a code block, you have to search for the address, which can be quite large. This can mean more typing sometimes. This can be fixed by giving code blocks some sort of relatively strong, but also relatively compact, hash. We can compute it lazily so that it only impacts run-time when debug support is enabled. We can stringify it smartly so that it's short and easy to type. We can base it on the source code so that the optimization level is irrelevant. And, we can use SHA1 since it's already in our code base. As part of this, we should also get CodeBlock debugging to use the new world of print() and dataLog(). In particular, CodeBlock::dump() should correspond to the thing you want printed if you do: dataLog("I heart ", *myCodeBlock); Probably, you want to just print some identifying information at this point rather than the full bytecode dump. This implies renaming the existing CodeBlock::dump() to CodeBlock::dumpBytecode(). Finally, this new notion of CodeBlockHash should be available for other purposes like bisecting breakage. So it should have all of the comparisons.
Created attachment 176689 [details] work in progress
Attachment 176689 [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/DFGOSRExitCompiler.cpp:104: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/bytecode/CodeOrigin.h:29: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:708: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:782: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:853: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:919: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:992: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:1070: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:1144: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/runtime/CodeSpecializationKind.h:48: The parameter name "kind" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JITDisassembler.cpp:31: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/jit/JITPropertyAccess.cpp:751: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess.cpp:820: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess.cpp:821: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess.cpp:889: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess.cpp:954: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess.cpp:1028: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess.cpp:1105: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/jit/JITPropertyAccess.cpp:1180: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/runtime/Executable.h:91: The parameter name "kind" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Executable.h:301: The parameter name "kind" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Executable.h:361: The parameter name "kind" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGRepatch.cpp:240: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGRepatch.cpp:301: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGRepatch.cpp:530: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGRepatch.cpp:742: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/JavaScriptCore/dfg/DFGRepatch.cpp:944: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 27 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 176689 [details] work in progress Attachment 176689 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/15026796
Comment on attachment 176689 [details] work in progress Attachment 176689 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/15023780
Comment on attachment 176689 [details] work in progress Attachment 176689 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15027775
Comment on attachment 176689 [details] work in progress Attachment 176689 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/15018806
Created attachment 176696 [details] sample disassembly Sample disassembly, of bitops-bitwise-and. Note that all of the things related to the benchmark's sole code block can be found by searching for DiBSvy. That's the code block hash of this benchmark's global code. It will not change from execution to execution, and so can be used for both casual introspection as well as bisecting.
Comment on attachment 176689 [details] work in progress Attachment 176689 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15016902
Comment on attachment 176689 [details] work in progress Attachment 176689 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15028977
Created attachment 176821 [details] the patch
Comment on attachment 176821 [details] the patch Attachment 176821 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/15060023
Comment on attachment 176821 [details] the patch Attachment 176821 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/15025900
Comment on attachment 176821 [details] the patch Attachment 176821 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/15057069
Comment on attachment 176821 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=176821&action=review r=me > Source/JavaScriptCore/bytecode/CodeBlock.cpp:72 > + out.print(hash(), ":[", RawPointer(this), ", ", jitType, codeType()); I wonder if there's a better way to call this out as a hash. Prefix with "#"?
Comment on attachment 176821 [details] the patch Attachment 176821 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15056076
(In reply to comment #14) > (From update of attachment 176821 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=176821&action=review > > r=me > > > Source/JavaScriptCore/bytecode/CodeBlock.cpp:72 > > + out.print(hash(), ":[", RawPointer(this), ", ", jitType, codeType()); > > I wonder if there's a better way to call this out as a hash. Prefix with "#"? Yeah, that's a good idea. I made all of the people who print CodeBlockHash's prefix them with #. This preserves the invariant that CodeBlockHash(toCString(foo).data()) == foo and !strcmp(toCString(CodeBlockHash(foo)).data(), foo)
Created attachment 176828 [details] patch for landing
Comment on attachment 176828 [details] patch for landing Attachment 176828 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/15044477
Comment on attachment 176828 [details] patch for landing Attachment 176828 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15044476
Created attachment 176877 [details] try again
Landed in http://trac.webkit.org/changeset/136199