Skip to content

Commit dba13b1

Browse files
committed
Bug 1674452: Add interface information to ORPC profiler markers; r=Jamie
* We add a new function to `mscom/Utils.h`: `DiagnosticNameForIID`. Its purpose is to generate a friendly name for an interface, given an IID. For special interfaces internal to COM, we add our own descriptive strings. If the interface does not have a description, we simply convert the IID to string format using GUIDToString. * We modify `ProfilerMarkerChannelHook` to include the additional diagnostic information for IIDs in its markers. * Since each marker is now differentiated by IID, we remove the restriction that we only use markers for the outermost COM call. In particular, this assumption doesn't hold for asynchronous COM calls, so we would be losing data in the case where an async call was pending while the main thread was still making COM calls on other interfaces. * There isn't really an effective way to distinguish between sync and async calls at the channel hook layer. I'm thinking about how we could perhaps modify `AsyncInvoker` to help mark these, but it's a bit messy. I'm going to postpone that to future work. * Other potential future work is expanding the number of interfaces for which we have frendly names. I could see us annotating our various COM interfaces in a way that we could automagically generate human-readable descriptions for those interfaces. Differential Revision: https://phabricator.services.mozilla.com/D97042
1 parent c3b7359 commit dba13b1

File tree

3 files changed

+111
-18
lines changed

3 files changed

+111
-18
lines changed

ipc/mscom/ProfilerMarkers.cpp

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@
1111
#include "mozilla/Assertions.h"
1212
#include "mozilla/Atomics.h"
1313
#include "mozilla/DebugOnly.h"
14+
#include "mozilla/mscom/Utils.h"
1415
#include "mozilla/Services.h"
1516
#include "nsCOMPtr.h"
1617
#include "nsIObserver.h"
1718
#include "nsIObserverService.h"
1819
#include "nsISupportsImpl.h"
20+
#include "nsString.h"
1921
#include "nsXULAppAPI.h"
2022

2123
#include <objbase.h>
@@ -31,7 +33,7 @@ class ProfilerMarkerChannelHook final : public IChannelHook {
3133
~ProfilerMarkerChannelHook() = default;
3234

3335
public:
34-
ProfilerMarkerChannelHook() : mRefCnt(0), mMainThreadORPCDepth(0) {}
36+
ProfilerMarkerChannelHook() : mRefCnt(0) {}
3537

3638
// IUnknown
3739
STDMETHODIMP QueryInterface(REFIID aIid, void** aOutInterface) override;
@@ -58,10 +60,6 @@ class ProfilerMarkerChannelHook final : public IChannelHook {
5860
*
5961
* Finally, our implementation responds to any request matching our extension
6062
* ID, however we only care about main thread COM calls.
61-
*
62-
* Further note that COM allows re-entrancy, however for our purposes we only
63-
* care about the outermost IPC call on the main thread, so we use the
64-
* mMainThreadORPCDepth variable to track that.
6563
*/
6664

6765
// IChannelHook
@@ -91,9 +89,11 @@ class ProfilerMarkerChannelHook final : public IChannelHook {
9189
ServerFillBuffer(REFGUID aExtensionId, REFIID aIid, ULONG* aDataSize,
9290
void* aDataBuf, HRESULT aFault) override {}
9391

92+
private:
93+
void BuildMarkerName(REFIID aIid, nsACString& aOutMarkerName);
94+
9495
private:
9596
mozilla::Atomic<ULONG> mRefCnt;
96-
ULONG mMainThreadORPCDepth;
9797
};
9898

9999
HRESULT ProfilerMarkerChannelHook::QueryInterface(REFIID aIid,
@@ -118,16 +118,23 @@ ULONG ProfilerMarkerChannelHook::Release() {
118118
return result;
119119
}
120120

121+
void ProfilerMarkerChannelHook::BuildMarkerName(REFIID aIid,
122+
nsACString& aOutMarkerName) {
123+
aOutMarkerName.AssignLiteral("ORPC Call for ");
124+
125+
nsAutoCString iidStr;
126+
mozilla::mscom::DiagnosticNameForIID(aIid, iidStr);
127+
aOutMarkerName.Append(iidStr);
128+
}
129+
121130
void ProfilerMarkerChannelHook::ClientGetSize(REFGUID aExtensionId, REFIID aIid,
122131
ULONG* aOutDataSize) {
123132
if (aExtensionId == GUID_MozProfilerMarkerExtension) {
124133
if (NS_IsMainThread()) {
125-
if (!mMainThreadORPCDepth) {
126-
PROFILER_TRACING_MARKER("MSCOM", "ORPC Call", IPC,
127-
TRACING_INTERVAL_START);
128-
}
129-
130-
++mMainThreadORPCDepth;
134+
nsAutoCString markerName;
135+
BuildMarkerName(aIid, markerName);
136+
PROFILER_TRACING_MARKER("MSCOM", markerName.get(), IPC,
137+
TRACING_INTERVAL_START);
131138
}
132139

133140
if (aOutDataSize) {
@@ -140,12 +147,11 @@ void ProfilerMarkerChannelHook::ClientGetSize(REFGUID aExtensionId, REFIID aIid,
140147
void ProfilerMarkerChannelHook::ClientNotify(REFGUID aExtensionId, REFIID aIid,
141148
ULONG aDataSize, void* aDataBuffer,
142149
DWORD aDataRep, HRESULT aFault) {
143-
if (aExtensionId == GUID_MozProfilerMarkerExtension && NS_IsMainThread()) {
144-
MOZ_ASSERT(mMainThreadORPCDepth > 0);
145-
--mMainThreadORPCDepth;
146-
if (!mMainThreadORPCDepth) {
147-
PROFILER_TRACING_MARKER("MSCOM", "ORPC Call", IPC, TRACING_INTERVAL_END);
148-
}
150+
if (NS_IsMainThread() && aExtensionId == GUID_MozProfilerMarkerExtension) {
151+
nsAutoCString markerName;
152+
BuildMarkerName(aIid, markerName);
153+
PROFILER_TRACING_MARKER("MSCOM", markerName.get(), IPC,
154+
TRACING_INTERVAL_END);
149155
}
150156
}
151157

ipc/mscom/Utils.cpp

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
#include <shlwapi.h>
2929
#include <winnt.h>
3030

31+
#include <utility>
32+
3133
#if defined(_MSC_VER)
3234
extern "C" IMAGE_DOS_HEADER __ImageBase;
3335
#endif
@@ -317,6 +319,82 @@ void GUIDToString(REFGUID aGuid, nsAString& aOutString) {
317319
}
318320
}
319321

322+
// Undocumented IIDs that are relevant for diagnostic purposes
323+
static const IID IID_ISCMLocalActivator = {
324+
0x00000136,
325+
0x0000,
326+
0x0000,
327+
{0xC0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46}};
328+
static const IID IID_IRundown = {
329+
0x00000134,
330+
0x0000,
331+
0x0000,
332+
{0xC0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46}};
333+
static const IID IID_IRemUnknown = {
334+
0x00000131,
335+
0x0000,
336+
0x0000,
337+
{0xC0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46}};
338+
static const IID IID_IRemUnknown2 = {
339+
0x00000143,
340+
0x0000,
341+
0x0000,
342+
{0xC0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46}};
343+
344+
struct IIDToLiteralMapEntry {
345+
constexpr IIDToLiteralMapEntry(REFIID aIid, nsLiteralCString&& aStr)
346+
: mIid(aIid), mStr(std::forward<nsLiteralCString>(aStr)) {}
347+
348+
REFIID mIid;
349+
const nsLiteralCString mStr;
350+
};
351+
352+
/**
353+
* Given the name of an interface, the IID_ENTRY macro generates a pair
354+
* containing a reference to the interface ID and a stringified version of
355+
* the interface name.
356+
*
357+
* For example:
358+
*
359+
* {IID_ENTRY(IUnknown)}
360+
* is expanded to:
361+
* {IID_IUnknown, "IUnknown"_ns}
362+
*
363+
*/
364+
// clang-format off
365+
# define IID_ENTRY_STRINGIFY(iface) #iface##_ns
366+
# define IID_ENTRY(iface) IID_##iface, IID_ENTRY_STRINGIFY(iface)
367+
// clang-format on
368+
369+
// Mapping of selected IIDs to friendly, human readable descriptions for each
370+
// interface.
371+
static constexpr IIDToLiteralMapEntry sIidDiagStrs[] = {
372+
{IID_ENTRY(IUnknown)},
373+
{IID_IRemUnknown, "cross-apartment IUnknown"_ns},
374+
{IID_IRundown, "cross-apartment object management"_ns},
375+
{IID_ISCMLocalActivator, "out-of-process object instantiation"_ns},
376+
{IID_IRemUnknown2, "cross-apartment IUnknown"_ns}};
377+
378+
# undef IID_ENTRY
379+
# undef IID_ENTRY_STRINGIFY
380+
381+
void DiagnosticNameForIID(REFIID aIid, nsACString& aOutString) {
382+
// If the IID matches something in sIidDiagStrs, output its string.
383+
for (const auto& curEntry : sIidDiagStrs) {
384+
if (curEntry.mIid == aIid) {
385+
aOutString.Assign(curEntry.mStr);
386+
return;
387+
}
388+
}
389+
390+
// Otherwise just convert the IID to string form and output that.
391+
nsAutoString strIid;
392+
GUIDToString(aIid, strIid);
393+
394+
aOutString.AssignLiteral("IID ");
395+
AppendUTF16toUTF8(strIid, aOutString);
396+
}
397+
320398
#else
321399

322400
void GUIDToString(REFGUID aGuid,

ipc/mscom/Utils.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,15 @@ constexpr size_t kGuidRegFormatCharLenInclNul = 39;
105105
bool IsClassThreadAwareInprocServer(REFCLSID aClsid);
106106

107107
void GUIDToString(REFGUID aGuid, nsAString& aOutString);
108+
109+
/**
110+
* Converts an IID to a human-readable string for the purposes of diagnostic
111+
* tools such as the profiler. For some special cases, we output a friendly
112+
* string that describes the purpose of the interface. If no such description
113+
* exists, we simply fall back to outputting the IID as a string formatted by
114+
* GUIDToString().
115+
*/
116+
void DiagnosticNameForIID(REFIID aIid, nsACString& aOutString);
108117
#else
109118
void GUIDToString(REFGUID aGuid,
110119
wchar_t (&aOutBuf)[kGuidRegFormatCharLenInclNul]);

0 commit comments

Comments
 (0)