Make the CLI for the sampling profiler better for inlined call site indices
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 28 Jan 2017 01:04:06 +0000 (01:04 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 28 Jan 2017 01:04:06 +0000 (01:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=167482

Reviewed by Mark Lam.

This patches changes the command line interface for the sampling
profiler to also dump the machine frame that the semantic code
origin is in if the semantic code origin is inlined. This helps
when doing performance work because it's helpful to know the
context that an inlined frame is in. Before, we used to just
say it was in the baseline JIT if it didn't have its own optimized
compile. Now, we can tell that its inlined into a DFG or FTL frame.

* inspector/agents/InspectorScriptProfilerAgent.cpp:
(Inspector::buildSamples):
* runtime/Options.h:
* runtime/SamplingProfiler.cpp:
(JSC::SamplingProfiler::processUnverifiedStackTraces):
(JSC::SamplingProfiler::reportTopFunctions):
(JSC::SamplingProfiler::reportTopBytecodes):
* runtime/SamplingProfiler.h:
(JSC::SamplingProfiler::StackFrame::CodeLocation::hasCodeBlockHash):
(JSC::SamplingProfiler::StackFrame::CodeLocation::hasBytecodeIndex):
(JSC::SamplingProfiler::StackFrame::CodeLocation::hasExpressionInfo):
(JSC::SamplingProfiler::StackFrame::hasExpressionInfo):
(JSC::SamplingProfiler::StackFrame::lineNumber):
(JSC::SamplingProfiler::StackFrame::columnNumber):
(JSC::SamplingProfiler::StackFrame::hasBytecodeIndex): Deleted.
(JSC::SamplingProfiler::StackFrame::hasCodeBlockHash): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@211316 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/inspector/agents/InspectorScriptProfilerAgent.cpp
Source/JavaScriptCore/runtime/Options.h
Source/JavaScriptCore/runtime/SamplingProfiler.cpp
Source/JavaScriptCore/runtime/SamplingProfiler.h

index 1375ee6..a1249d5 100644 (file)
@@ -1,3 +1,35 @@
+2017-01-27  Saam Barati  <sbarati@apple.com>
+
+        Make the CLI for the sampling profiler better for inlined call site indices
+        https://bugs.webkit.org/show_bug.cgi?id=167482
+
+        Reviewed by Mark Lam.
+
+        This patches changes the command line interface for the sampling
+        profiler to also dump the machine frame that the semantic code
+        origin is in if the semantic code origin is inlined. This helps
+        when doing performance work because it's helpful to know the
+        context that an inlined frame is in. Before, we used to just
+        say it was in the baseline JIT if it didn't have its own optimized
+        compile. Now, we can tell that its inlined into a DFG or FTL frame.
+
+        * inspector/agents/InspectorScriptProfilerAgent.cpp:
+        (Inspector::buildSamples):
+        * runtime/Options.h:
+        * runtime/SamplingProfiler.cpp:
+        (JSC::SamplingProfiler::processUnverifiedStackTraces):
+        (JSC::SamplingProfiler::reportTopFunctions):
+        (JSC::SamplingProfiler::reportTopBytecodes):
+        * runtime/SamplingProfiler.h:
+        (JSC::SamplingProfiler::StackFrame::CodeLocation::hasCodeBlockHash):
+        (JSC::SamplingProfiler::StackFrame::CodeLocation::hasBytecodeIndex):
+        (JSC::SamplingProfiler::StackFrame::CodeLocation::hasExpressionInfo):
+        (JSC::SamplingProfiler::StackFrame::hasExpressionInfo):
+        (JSC::SamplingProfiler::StackFrame::lineNumber):
+        (JSC::SamplingProfiler::StackFrame::columnNumber):
+        (JSC::SamplingProfiler::StackFrame::hasBytecodeIndex): Deleted.
+        (JSC::SamplingProfiler::StackFrame::hasCodeBlockHash): Deleted.
+
 2017-01-27  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Extend create_hash_table to specify Intrinsic
index 0c95539..483adf8 100644 (file)
@@ -180,8 +180,8 @@ static Ref<Protocol::ScriptProfiler::Samples> buildSamples(VM& vm, Vector<Sampli
 
             if (stackFrame.hasExpressionInfo()) {
                 Ref<Protocol::ScriptProfiler::ExpressionLocation> expressionLocation = Protocol::ScriptProfiler::ExpressionLocation::create()
-                    .setLine(stackFrame.lineNumber)
-                    .setColumn(stackFrame.columnNumber)
+                    .setLine(stackFrame.lineNumber())
+                    .setColumn(stackFrame.columnNumber())
                     .release();
                 frame->setExpressionLocation(WTFMove(expressionLocation));
             }
index f3f6f46..7f53dc4 100644 (file)
@@ -361,6 +361,8 @@ typedef const char* optionString;
     v(bool, useSamplingProfiler, false, Normal, nullptr) \
     v(unsigned, sampleInterval, 1000, Normal, "Time between stack traces in microseconds.") \
     v(bool, collectSamplingProfilerDataForJSCShell, false, Normal, "This corresponds to the JSC shell's --sample option.") \
+    v(unsigned, samplingProfilerTopFunctionsCount, 12, Normal, "Number of top functions to report when using the command line interface.") \
+    v(unsigned, samplingProfilerTopBytecodesCount, 40, Normal, "Number of top bytecodes to report when using the command line interface.") \
     v(optionString, samplingProfilerPath, nullptr, Normal, "The path to the directory to write sampiling profiler output to. This probably will not work with WK2 unless the path is in the whitelist.") \
     \
     v(bool, alwaysGeneratePCToCodeOriginMap, false, Normal, "This will make sure we always generate a PCToCodeOriginMap for JITed code.") \
index 8bf2174..2e07a04 100644 (file)
@@ -44,6 +44,7 @@
 #include "NativeExecutable.h"
 #include "PCToCodeOriginMap.h"
 #include "SlotVisitor.h"
+#include "StrongInlines.h"
 #include "VM.h"
 #include <wtf/HashSet.h>
 #include <wtf/RandomNumber.h>
@@ -363,29 +364,32 @@ void SamplingProfiler::processUnverifiedStackTraces()
         StackTrace& stackTrace = m_stackTraces.last();
         stackTrace.timestamp = unprocessedStackTrace.timestamp;
 
-        auto appendCodeBlock = [&] (CodeBlock* codeBlock, unsigned bytecodeIndex) {
-            stackTrace.frames.append(StackFrame(codeBlock->ownerExecutable()));
-            m_liveCellPointers.add(codeBlock->ownerExecutable());
-
+        auto populateCodeLocation = [] (CodeBlock* codeBlock, unsigned bytecodeIndex, StackFrame::CodeLocation& location) {
             if (bytecodeIndex < codeBlock->instructionCount()) {
                 int divot;
                 int startOffset;
                 int endOffset;
                 codeBlock->expressionRangeForBytecodeOffset(bytecodeIndex, divot, startOffset, endOffset,
-                    stackTrace.frames.last().lineNumber, stackTrace.frames.last().columnNumber);
-                stackTrace.frames.last().bytecodeIndex = bytecodeIndex;
+                    location.lineNumber, location.columnNumber);
+                location.bytecodeIndex = bytecodeIndex;
             }
             if (Options::collectSamplingProfilerDataForJSCShell()) {
-                stackTrace.frames.last().codeBlockHash = codeBlock->hash();
-                stackTrace.frames.last().jitType = codeBlock->jitType();
+                location.codeBlockHash = codeBlock->hash();
+                location.jitType = codeBlock->jitType();
             }
         };
 
+        auto appendCodeBlock = [&] (CodeBlock* codeBlock, unsigned bytecodeIndex) {
+            stackTrace.frames.append(StackFrame(codeBlock->ownerExecutable()));
+            m_liveCellPointers.add(codeBlock->ownerExecutable());
+            populateCodeLocation(codeBlock, bytecodeIndex, stackTrace.frames.last().semanticLocation);
+        };
+
         auto appendEmptyFrame = [&] {
             stackTrace.frames.append(StackFrame());
         };
 
-        auto storeCalleeIntoTopFrame = [&] (EncodedJSValue encodedCallee) {
+        auto storeCalleeIntoLastFrame = [&] (EncodedJSValue encodedCallee) {
             // Set the callee if it's a valid GC object.
             JSValue callee = JSValue::decode(encodedCallee);
             StackFrame& stackFrame = stackTrace.frames.last();
@@ -441,6 +445,30 @@ void SamplingProfiler::processUnverifiedStackTraces()
             m_liveCellPointers.add(executable);
         };
 
+        auto appendCodeOrigin = [&] (CodeBlock* machineCodeBlock, CodeOrigin origin) {
+            size_t startIndex = stackTrace.frames.size(); // We want to change stack traces that we're about to append.
+
+            CodeOrigin machineOrigin;
+            origin.walkUpInlineStack([&] (const CodeOrigin& codeOrigin) {
+                machineOrigin = codeOrigin;
+                appendCodeBlock(codeOrigin.inlineCallFrame ? codeOrigin.inlineCallFrame->baselineCodeBlock.get() : machineCodeBlock, codeOrigin.bytecodeIndex);
+            });
+
+            if (Options::collectSamplingProfilerDataForJSCShell()) {
+                RELEASE_ASSERT(machineOrigin.isSet());
+                RELEASE_ASSERT(!machineOrigin.inlineCallFrame);
+
+                StackFrame::CodeLocation machineLocation = stackTrace.frames.last().semanticLocation;
+
+                // We want to tell each inlined frame about the machine frame
+                // they were inlined into. Currently, we only use this for dumping
+                // output on the command line, but we could extend it to the web
+                // inspector in the future if we find a need for it there.
+                RELEASE_ASSERT(stackTrace.frames.size());
+                for (size_t i = startIndex; i < stackTrace.frames.size() - 1; i++)
+                    stackTrace.frames[i].machineLocation = std::make_pair(machineLocation, Strong<CodeBlock>(m_vm, machineCodeBlock));
+            }
+        };
 
         // Prepend the top-most inlined frame if needed and gather
         // location information about where the top frame is executing.
@@ -468,14 +496,12 @@ void SamplingProfiler::processUnverifiedStackTraces()
                     UNUSED_PARAM(isValidPC); // FIXME: do something with this info for the web inspector: https://bugs.webkit.org/show_bug.cgi?id=153455
 
                     appendCodeBlock(topCodeBlock, bytecodeIndex);
-                    storeCalleeIntoTopFrame(unprocessedStackTrace.frames[0].unverifiedCallee);
+                    storeCalleeIntoLastFrame(unprocessedStackTrace.frames[0].unverifiedCallee);
                     startIndex = 1;
                 }
             } else if (std::optional<CodeOrigin> codeOrigin = topCodeBlock->findPC(unprocessedStackTrace.topPC)) {
-                codeOrigin->walkUpInlineStack([&] (const CodeOrigin& codeOrigin) {
-                    appendCodeBlock(codeOrigin.inlineCallFrame ? codeOrigin.inlineCallFrame->baselineCodeBlock.get() : topCodeBlock, codeOrigin.bytecodeIndex);
-                });
-                storeCalleeIntoTopFrame(unprocessedStackTrace.frames[0].unverifiedCallee);
+                appendCodeOrigin(topCodeBlock, *codeOrigin);
+                storeCalleeIntoLastFrame(unprocessedStackTrace.frames[0].unverifiedCallee);
                 startIndex = 1;
             }
         }
@@ -492,11 +518,9 @@ void SamplingProfiler::processUnverifiedStackTraces()
 
 #if ENABLE(DFG_JIT)
                 if (codeBlock->hasCodeOrigins()) {
-                    if (codeBlock->canGetCodeOrigin(callSiteIndex)) {
-                        codeBlock->codeOrigin(callSiteIndex).walkUpInlineStack([&] (const CodeOrigin& codeOrigin) {
-                            appendCodeBlock(codeOrigin.inlineCallFrame ? codeOrigin.inlineCallFrame->baselineCodeBlock.get() : codeBlock, codeOrigin.bytecodeIndex);
-                        });
-                    } else
+                    if (codeBlock->canGetCodeOrigin(callSiteIndex))
+                        appendCodeOrigin(codeBlock, codeBlock->codeOrigin(callSiteIndex));
+                    else
                         appendCodeBlock(codeBlock, std::numeric_limits<unsigned>::max());
                 } else
                     appendCodeBlockNoInlining();
@@ -508,7 +532,7 @@ void SamplingProfiler::processUnverifiedStackTraces()
 
             // Note that this is okay to do if we walked the inline stack because
             // the machine frame will be at the top of the processed stack trace.
-            storeCalleeIntoTopFrame(unprocessedStackFrame.unverifiedCallee);
+            storeCalleeIntoLastFrame(unprocessedStackFrame.unverifiedCallee);
         }
     }
 
@@ -843,14 +867,16 @@ void SamplingProfiler::reportTopFunctions(PrintStream& out)
         return std::make_pair(maxFrameDescription, maxFrameCount);
     };
 
-    out.print("\n\nSampling rate: ", m_timingInterval.count(), " microseconds\n");
-    out.print("Hottest functions as <numSamples  'functionName:sourceID'>\n");
-    for (size_t i = 0; i < 40; i++) {
-        auto pair = takeMax();
-        if (pair.first.isEmpty())
-            break;
-        out.printf("%6zu ", pair.second);
-        out.print("   '", pair.first, "'\n");
+    if (Options::samplingProfilerTopFunctionsCount()) {
+        out.print("\n\nSampling rate: ", m_timingInterval.count(), " microseconds\n");
+        out.print("Top functions as <numSamples  'functionName:sourceID'>\n");
+        for (size_t i = 0; i < Options::samplingProfilerTopFunctionsCount(); i++) {
+            auto pair = takeMax();
+            if (pair.first.isEmpty())
+                break;
+            out.printf("%6zu ", pair.second);
+            out.print("   '", pair.first, "'\n");
+        }
     }
 }
 
@@ -873,22 +899,30 @@ void SamplingProfiler::reportTopBytecodes(PrintStream& out)
         if (!stackTrace.frames.size())
             continue;
 
+        auto descriptionForLocation = [&] (StackFrame::CodeLocation location) -> String {
+            String bytecodeIndex;
+            String codeBlockHash;
+            if (location.hasBytecodeIndex())
+                bytecodeIndex = String::number(location.bytecodeIndex);
+            else
+                bytecodeIndex = "<nil>";
+
+            if (location.hasCodeBlockHash()) {
+                StringPrintStream stream;
+                location.codeBlockHash.dump(stream);
+                codeBlockHash = stream.toString();
+            } else
+                codeBlockHash = "<nil>";
+
+            return makeString("#", codeBlockHash, ":", JITCode::typeName(location.jitType), ":", bytecodeIndex);
+        };
+
         StackFrame& frame = stackTrace.frames.first();
-        String bytecodeIndex;
-        String codeBlockHash;
-        if (frame.hasBytecodeIndex())
-            bytecodeIndex = String::number(frame.bytecodeIndex);
-        else
-            bytecodeIndex = "<nil>";
-
-        if (frame.hasCodeBlockHash()) {
-            StringPrintStream stream;
-            frame.codeBlockHash.dump(stream);
-            codeBlockHash = stream.toString();
-        } else
-            codeBlockHash = "<nil>";
-
-        String frameDescription = makeString(frame.displayName(m_vm), "#", codeBlockHash, ":", JITCode::typeName(frame.jitType), ":", bytecodeIndex);
+        String frameDescription = makeString(frame.displayName(m_vm), descriptionForLocation(frame.semanticLocation));
+        if (std::optional<std::pair<StackFrame::CodeLocation, Strong<CodeBlock>>> machineLocation = frame.machineLocation) {
+            frameDescription = makeString(frameDescription, " <-- ",
+                machineLocation->second->inferredName().data(), descriptionForLocation(machineLocation->first));
+        }
         bytecodeCounts.add(frameDescription, 0).iterator->value++;
     }
 
@@ -906,14 +940,16 @@ void SamplingProfiler::reportTopBytecodes(PrintStream& out)
         return std::make_pair(maxFrameDescription, maxFrameCount);
     };
 
-    out.print("\n\nSampling rate: ", m_timingInterval.count(), " microseconds\n");
-    out.print("Hottest bytecodes as <numSamples   'functionName#hash:JITType:bytecodeIndex'>\n");
-    for (size_t i = 0; i < 80; i++) {
-        auto pair = takeMax();
-        if (pair.first.isEmpty())
-            break;
-        out.printf("%6zu ", pair.second);
-        out.print("   '", pair.first, "'\n");
+    if (Options::samplingProfilerTopBytecodesCount()) {
+        out.print("\n\nSampling rate: ", m_timingInterval.count(), " microseconds\n");
+        out.print("Hottest bytecodes as <numSamples   'functionName#hash:JITType:bytecodeIndex'>\n");
+        for (size_t i = 0; i < Options::samplingProfilerTopBytecodesCount(); i++) {
+            auto pair = takeMax();
+            if (pair.first.isEmpty())
+                break;
+            out.printf("%6zu ", pair.second);
+            out.print("   '", pair.first, "'\n");
+        }
     }
 }
 
index c92de6f..029cc70 100644 (file)
@@ -80,27 +80,45 @@ public:
         FrameType frameType { FrameType::Unknown };
         ExecutableBase* executable { nullptr };
         JSObject* callee { nullptr };
-        // These attempt to be expression-level line and column number.
-        unsigned lineNumber { std::numeric_limits<unsigned>::max() };
-        unsigned columnNumber { std::numeric_limits<unsigned>::max() };
-        unsigned bytecodeIndex { std::numeric_limits<unsigned>::max() };
-        CodeBlockHash codeBlockHash;
-        JITCode::JITType jitType { JITCode::None };
-
-        bool hasExpressionInfo() const
-        {
-            return lineNumber != std::numeric_limits<unsigned>::max()
-                && columnNumber != std::numeric_limits<unsigned>::max();
-        }
 
-        bool hasBytecodeIndex() const
+        struct CodeLocation {
+            bool hasCodeBlockHash() const
+            {
+                return codeBlockHash.isSet();
+            }
+
+            bool hasBytecodeIndex() const
+            {
+                return bytecodeIndex != std::numeric_limits<unsigned>::max();
+            }
+
+            bool hasExpressionInfo() const
+            {
+                return lineNumber != std::numeric_limits<unsigned>::max()
+                    && columnNumber != std::numeric_limits<unsigned>::max();
+            }
+
+            // These attempt to be expression-level line and column number.
+            unsigned lineNumber { std::numeric_limits<unsigned>::max() };
+            unsigned columnNumber { std::numeric_limits<unsigned>::max() };
+            unsigned bytecodeIndex { std::numeric_limits<unsigned>::max() };
+            CodeBlockHash codeBlockHash;
+            JITCode::JITType jitType { JITCode::None };
+        };
+
+        CodeLocation semanticLocation;
+        std::optional<std::pair<CodeLocation, Strong<CodeBlock>>> machineLocation; // This is non-null if we were inlined. It represents the machine frame we were inlined into.
+
+        bool hasExpressionInfo() const { return semanticLocation.hasExpressionInfo(); }
+        unsigned lineNumber() const
         {
-            return bytecodeIndex != std::numeric_limits<unsigned>::max();
+            ASSERT(hasExpressionInfo());
+            return semanticLocation.lineNumber;
         }
-
-        bool hasCodeBlockHash() const
+        unsigned columnNumber() const
         {
-            return codeBlockHash.isSet();
+            ASSERT(hasExpressionInfo());
+            return semanticLocation.columnNumber;
         }
 
         // These are function-level data.