Bug 1405541 - Split AUTO_PROFILER_LABEL_DYNAMIC into three macros. r=mstange.

It's easy to mess up the scoping so that (a) the label is pushed and then
immediately popped, and/or (b) the string doesn't live long enough. It's also
easy to do a utf16-to-utf8 conversion unnecessarily when the profiler is
inactive.

This patch splits that macro into three new ones that are harder to mess up.

- AUTO_PROFILER_LABEL_DYNAMIC_CSTR: same as current.
- AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING: for nsCStrings.
- AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING: for nsStrings.

--HG--
extra : rebase_source : 53c8b43b6a1be06d00618a133e28bf95c46a3ba3
This commit is contained in:
Nicholas Nethercote 2017-10-11 13:03:34 +02:00
parent 748a976761
commit add7e65972
18 changed files with 105 additions and 89 deletions

View File

@ -600,13 +600,8 @@ nsFrameMessageManager::SendMessage(const nsAString& aMessageName,
JS::MutableHandle<JS::Value> aRetval,
bool aIsSync)
{
#ifdef MOZ_GECKO_PROFILER
if (profiler_is_active()) {
NS_LossyConvertUTF16toASCII messageNameCStr(aMessageName);
AUTO_PROFILER_LABEL_DYNAMIC("nsFrameMessageManager::SendMessage", EVENTS,
messageNameCStr.get());
}
#endif
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"nsFrameMessageManager::SendMessage", EVENTS, aMessageName);
NS_ASSERTION(!IsGlobal(), "Should not call SendSyncMessage in chrome");
NS_ASSERTION(!IsBroadcaster(), "Should not call SendSyncMessage in chrome");
@ -1541,14 +1536,8 @@ void
nsMessageManagerScriptExecutor::LoadScriptInternal(const nsAString& aURL,
bool aRunInGlobalScope)
{
#ifdef MOZ_GECKO_PROFILER
if (profiler_is_active()) {
NS_LossyConvertUTF16toASCII urlCStr(aURL);
AUTO_PROFILER_LABEL_DYNAMIC(
"nsMessageManagerScriptExecutor::LoadScriptInternal", OTHER,
urlCStr.get());
}
#endif
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"nsMessageManagerScriptExecutor::LoadScriptInternal", OTHER, aURL);
if (!mGlobal || !sCachedScripts) {
return;

View File

@ -1169,8 +1169,8 @@ nsJSContext::GarbageCollectNow(JS::gcreason::Reason aReason,
IsShrinking aShrinking,
int64_t aSliceMillis)
{
AUTO_PROFILER_LABEL_DYNAMIC("nsJSContext::GarbageCollectNow", GC,
JS::gcreason::ExplainReason(aReason));
AUTO_PROFILER_LABEL_DYNAMIC_CSTR("nsJSContext::GarbageCollectNow", GC,
JS::gcreason::ExplainReason(aReason));
MOZ_ASSERT_IF(aSliceMillis, aIncremental == IncrementalGC);

View File

@ -1273,10 +1273,8 @@ EventListenerManager::HandleEventInternal(nsPresContext* aPresContext,
// do this extra work when we're not profiling.
nsAutoString typeStr;
(*aDOMEvent)->GetType(typeStr);
NS_LossyConvertUTF16toASCII typeCStr(typeStr);
AUTO_PROFILER_LABEL_DYNAMIC(
"EventListenerManager::HandleEventInternal", EVENTS,
typeCStr.get());
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"EventListenerManager::HandleEventInternal", EVENTS, typeStr);
TimeStamp startTime = TimeStamp::Now();
rv = HandleEventSubType(listener, *aDOMEvent, aCurrentTarget);

View File

@ -2508,9 +2508,8 @@ ContentChild::RecvAsyncMessage(const nsString& aMsg,
const IPC::Principal& aPrincipal,
const ClonedMessageData& aData)
{
NS_LossyConvertUTF16toASCII messageNameCStr(aMsg);
AUTO_PROFILER_LABEL_DYNAMIC("ContentChild::RecvAsyncMessage", EVENTS,
messageNameCStr.get());
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"ContentChild::RecvAsyncMessage", EVENTS, aMsg);
CrossProcessCpowHolder cpows(this, aCpows);
RefPtr<nsFrameMessageManager> cpm =

View File

@ -2337,9 +2337,8 @@ TabChild::RecvAsyncMessage(const nsString& aMessage,
const IPC::Principal& aPrincipal,
const ClonedMessageData& aData)
{
NS_LossyConvertUTF16toASCII messageNameCStr(aMessage);
AUTO_PROFILER_LABEL_DYNAMIC("TabChild::RecvAsyncMessage", EVENTS,
messageNameCStr.get());
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"TabChild::RecvAsyncMessage", EVENTS, aMessage);
CrossProcessCpowHolder cpows(Manager(), aCpows);
if (!mTabChildGlobal) {

View File

@ -1707,9 +1707,8 @@ TabParent::RecvSyncMessage(const nsString& aMessage,
const IPC::Principal& aPrincipal,
nsTArray<StructuredCloneData>* aRetVal)
{
NS_LossyConvertUTF16toASCII messageNameCStr(aMessage);
AUTO_PROFILER_LABEL_DYNAMIC("TabParent::RecvSyncMessage", EVENTS,
messageNameCStr.get());
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"TabParent::RecvSyncMessage", EVENTS, aMessage);
StructuredCloneData data;
ipc::UnpackClonedMessageDataForParent(aData, data);
@ -1728,9 +1727,8 @@ TabParent::RecvRpcMessage(const nsString& aMessage,
const IPC::Principal& aPrincipal,
nsTArray<StructuredCloneData>* aRetVal)
{
NS_LossyConvertUTF16toASCII messageNameCStr(aMessage);
AUTO_PROFILER_LABEL_DYNAMIC("TabParent::RecvRpcMessage", EVENTS,
messageNameCStr.get());
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"TabParent::RecvRpcMessage", EVENTS, aMessage);
StructuredCloneData data;
ipc::UnpackClonedMessageDataForParent(aData, data);
@ -1748,9 +1746,8 @@ TabParent::RecvAsyncMessage(const nsString& aMessage,
const IPC::Principal& aPrincipal,
const ClonedMessageData& aData)
{
NS_LossyConvertUTF16toASCII messageNameCStr(aMessage);
AUTO_PROFILER_LABEL_DYNAMIC("TabParent::RecvAsyncMessage", EVENTS,
messageNameCStr.get());
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"TabParent::RecvAsyncMessage", EVENTS, aMessage);
StructuredCloneData data;
ipc::UnpackClonedMessageDataForParent(aData, data);

View File

@ -172,9 +172,8 @@ nsIContentChild::RecvAsyncMessage(const nsString& aMsg,
const IPC::Principal& aPrincipal,
const ClonedMessageData& aData)
{
NS_LossyConvertUTF16toASCII messageNameCStr(aMsg);
AUTO_PROFILER_LABEL_DYNAMIC("nsIContentChild::RecvAsyncMessage", EVENTS,
messageNameCStr.get());
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"nsIContentChild::RecvAsyncMessage", EVENTS, aMsg);
CrossProcessCpowHolder cpows(this, aCpows);
RefPtr<nsFrameMessageManager> cpm = nsFrameMessageManager::GetChildProcessManager();

View File

@ -247,9 +247,8 @@ nsIContentParent::RecvSyncMessage(const nsString& aMsg,
const IPC::Principal& aPrincipal,
nsTArray<ipc::StructuredCloneData>* aRetvals)
{
NS_LossyConvertUTF16toASCII messageNameCStr(aMsg);
AUTO_PROFILER_LABEL_DYNAMIC("nsIContentParent::RecvSyncMessage", EVENTS,
messageNameCStr.get());
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"nsIContentParent::RecvSyncMessage", EVENTS, aMsg);
CrossProcessCpowHolder cpows(this, aCpows);
RefPtr<nsFrameMessageManager> ppm = mMessageManager;
@ -270,9 +269,8 @@ nsIContentParent::RecvRpcMessage(const nsString& aMsg,
const IPC::Principal& aPrincipal,
nsTArray<ipc::StructuredCloneData>* aRetvals)
{
NS_LossyConvertUTF16toASCII messageNameCStr(aMsg);
AUTO_PROFILER_LABEL_DYNAMIC("nsIContentParent::RecvRpcMessage", EVENTS,
messageNameCStr.get());
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"nsIContentParent::RecvRpcMessage", EVENTS, aMsg);
CrossProcessCpowHolder cpows(this, aCpows);
RefPtr<nsFrameMessageManager> ppm = mMessageManager;
@ -331,9 +329,8 @@ nsIContentParent::RecvAsyncMessage(const nsString& aMsg,
const IPC::Principal& aPrincipal,
const ClonedMessageData& aData)
{
NS_LossyConvertUTF16toASCII messageNameCStr(aMsg);
AUTO_PROFILER_LABEL_DYNAMIC("nsIContentParent::RecvAsyncMessage", EVENTS,
messageNameCStr.get());
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"nsIContentParent::RecvAsyncMessage", EVENTS, aMsg);
CrossProcessCpowHolder cpows(this, aCpows);
RefPtr<nsFrameMessageManager> ppm = mMessageManager;

View File

@ -319,8 +319,8 @@ DecodePool::SyncRunIfPreferred(IDecodingTask* aTask, const nsCString& aURI)
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(aTask);
AUTO_PROFILER_LABEL_DYNAMIC("DecodePool::SyncRunIfPreferred", GRAPHICS,
aURI.get());
AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(
"DecodePool::SyncRunIfPreferred", GRAPHICS, aURI);
if (aTask->ShouldPreferSyncRun()) {
aTask->Run();
@ -337,8 +337,8 @@ DecodePool::SyncRunIfPossible(IDecodingTask* aTask, const nsCString& aURI)
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(aTask);
AUTO_PROFILER_LABEL_DYNAMIC("DecodePool::SyncRunIfPossible", GRAPHICS,
aURI.get());
AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(
"DecodePool::SyncRunIfPossible", GRAPHICS, aURI);
aTask->Run();
}

View File

@ -369,8 +369,8 @@ mozJSComponentLoader::LoadModule(FileLocation& aFile)
return nullptr;
}
AUTO_PROFILER_LABEL_DYNAMIC("mozJSComponentLoader::LoadModule", OTHER,
spec.get());
AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(
"mozJSComponentLoader::LoadModule", OTHER, spec);
ModuleEntry* mod;
if (mModules.Get(spec, &mod))

View File

@ -641,10 +641,9 @@ mozJSSubScriptLoader::DoLoadSubScriptWithOptions(const nsAString& url,
return NS_OK;
}
const nsCString& asciiUrl = NS_LossyConvertUTF16toASCII(url);
AUTO_PROFILER_LABEL_DYNAMIC(
"mozJSSubScriptLoader::DoLoadSubScriptWithOptions", OTHER,
asciiUrl.get());
NS_LossyConvertUTF16toASCII asciiUrl(url);
AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(
"mozJSSubScriptLoader::DoLoadSubScriptWithOptions", OTHER, asciiUrl);
// Make sure to explicitly create the URI, since we'll need the
// canonicalized spec.

View File

@ -2504,11 +2504,8 @@ nsXPCComponents_Utils::Import(const nsACString& registryLocation,
RefPtr<mozJSComponentLoader> moduleloader = mozJSComponentLoader::Get();
MOZ_ASSERT(moduleloader);
#ifdef MOZ_GECKO_PROFILER
const nsCString& flatLocation = PromiseFlatCString(registryLocation);
AUTO_PROFILER_LABEL_DYNAMIC("nsXPCComponents_Utils::Import", OTHER,
flatLocation.get());
#endif
AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(
"nsXPCComponents_Utils::Import", OTHER, registryLocation);
return moduleloader->Import(registryLocation, targetObj, cx, optionalArgc, retval);
}

View File

@ -4049,8 +4049,8 @@ PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush aFlush)
"Layout",
"Display"
};
AUTO_PROFILER_LABEL_DYNAMIC("PresShell::DoFlushPendingNotifications",
GRAPHICS, flushTypeNames[flushType]);
AUTO_PROFILER_LABEL_DYNAMIC_CSTR("PresShell::DoFlushPendingNotifications",
GRAPHICS, flushTypeNames[flushType]);
#endif
#ifdef ACCESSIBILITY
@ -6269,13 +6269,16 @@ PresShell::Paint(nsView* aViewToPaint,
const nsRegion& aDirtyRegion,
uint32_t aFlags)
{
#ifdef MOZ_GECKO_PROFILER
nsIURI* uri = mDocument->GetDocumentURI();
nsIDocument* contentRoot = GetPrimaryContentDocument();
if (contentRoot) {
uri = contentRoot->GetDocumentURI();
}
nsCString uriString = uri ? uri->GetSpecOrDefault() : NS_LITERAL_CSTRING("N/A");
AUTO_PROFILER_LABEL_DYNAMIC("PresShell::Paint", GRAPHICS, uriString.get());
AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(
"PresShell::Paint", GRAPHICS,
uri ? uri->GetSpecOrDefault() : NS_LITERAL_CSTRING("N/A"));
#endif
Maybe<js::AutoAssertNoContentJS> nojs;
@ -8849,9 +8852,12 @@ PresShell::DoReflow(nsIFrame* target, bool aInterruptible)
parent = nsLayoutUtils::GetCrossDocParentFrame(parent);
}
#ifdef MOZ_GECKO_PROFILER
nsIURI* uri = mDocument->GetDocumentURI();
nsCString uriString = uri ? uri->GetSpecOrDefault() : NS_LITERAL_CSTRING("N/A");
AUTO_PROFILER_LABEL_DYNAMIC("PresShell::DoReflow", GRAPHICS, uriString.get());
AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(
"PresShell::DoReflow", GRAPHICS,
uri ? uri->GetSpecOrDefault() : NS_LITERAL_CSTRING("N/A"));
#endif
nsDocShell* docShell = static_cast<nsDocShell*>(GetPresContext()->GetDocShell());
RefPtr<TimelineConsumers> timelines = TimelineConsumers::Get();

View File

@ -116,8 +116,8 @@ RestyleTracker::DoProcessRestyles()
docURL = uri->GetSpecOrDefault();
}
}
AUTO_PROFILER_LABEL_DYNAMIC("RestyleTracker::DoProcessRestyles", CSS,
docURL.get());
AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(
"RestyleTracker::DoProcessRestyles", CSS, docURL);
#endif
// Create a AnimationsWithDestroyedFrame during restyling process to

View File

@ -6010,8 +6010,8 @@ FrameLayerBuilder::PaintItems(nsTArray<ClippedDisplayItem>& aItems,
continue;
#ifdef MOZ_DUMP_PAINTING
AUTO_PROFILER_LABEL_DYNAMIC("FrameLayerBuilder::PaintItems", GRAPHICS,
cdi->mItem->Name());
AUTO_PROFILER_LABEL_DYNAMIC_CSTR("FrameLayerBuilder::PaintItems", GRAPHICS,
cdi->mItem->Name());
#else
AUTO_PROFILER_LABEL("FrameLayerBuilder::PaintItems", GRAPHICS);
#endif

View File

@ -40,7 +40,9 @@
#define PROFILER_FEATURE_ACTIVE(feature) false
#define AUTO_PROFILER_LABEL(label, category)
#define AUTO_PROFILER_LABEL_DYNAMIC(label, category, dynamicStr)
#define AUTO_PROFILER_LABEL_DYNAMIC_CSTR(label, category, cStr)
#define AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(label, category, nsCStr)
#define AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(label, category, nsStr)
#define PROFILER_ADD_MARKER(markerName)
@ -391,22 +393,25 @@ PseudoStack* profiler_get_pseudo_stack();
// Insert an RAII object in this scope to enter a pseudo stack frame. Any
// samples collected in this scope will contain this label in their pseudo
// stack. The label argument must be a string literal. It is usually of the
// stack. The label argument must be a static C string. It is usually of the
// form "ClassName::FunctionName". (Ideally we'd use the compiler to provide
// that for us, but __func__ gives us the function name without the class
// name.) If the label applies to only part of a function, you can qualify it
// like this: "ClassName::FunctionName:PartName".
//
// Use AUTO_PROFILER_LABEL_DYNAMIC if you want to add additional / dynamic
// Use AUTO_PROFILER_LABEL_DYNAMIC_* if you want to add additional / dynamic
// information to the pseudo stack frame.
#define AUTO_PROFILER_LABEL(label, category) \
mozilla::AutoProfilerLabel PROFILER_RAII(label, nullptr, __LINE__, \
js::ProfileEntry::Category::category)
// Similar to AUTO_PROFILER_LABEL, but with an additional string. The inserted
// RAII object stores the dynamicStr pointer in a field; it does not copy the
// string. This means that the string you pass to this macro needs to live at
// least until the end of the current scope.
// RAII object stores the cStr pointer in a field; it does not copy the string.
//
// WARNING: This means that the string you pass to this macro needs to live at
// least until the end of the current scope. Be careful using this macro with
// ns[C]String; the other AUTO_PROFILER_LABEL_DYNAMIC_* macros below are
// preferred because they avoid this problem.
//
// If the profiler samples the current thread and walks the pseudo stack while
// this RAII object is on the stack, it will copy the supplied string into the
@ -418,10 +423,37 @@ PseudoStack* profiler_get_pseudo_stack();
// AUTO_PROFILER_LABEL are sampled, no string copy needs to be made because the
// profile buffer can just store the raw pointers to the literal strings.
// Consequently, AUTO_PROFILER_LABEL frames take up considerably less space in
// the profile buffer than AUTO_PROFILER_LABEL_DYNAMIC frames.
#define AUTO_PROFILER_LABEL_DYNAMIC(label, category, dynamicStr) \
mozilla::AutoProfilerLabel PROFILER_RAII(label, dynamicStr, __LINE__, \
js::ProfileEntry::Category::category)
// the profile buffer than AUTO_PROFILER_LABEL_DYNAMIC_* frames.
#define AUTO_PROFILER_LABEL_DYNAMIC_CSTR(label, category, cStr) \
mozilla::AutoProfilerLabel \
PROFILER_RAII(label, cStr, __LINE__, js::ProfileEntry::Category::category)
// Similar to AUTO_PROFILER_LABEL_DYNAMIC_CSTR, but takes an nsACString.
//
// Note: The use of the reference ensures that the dynamic string lives long
// enough.
#define AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(label, category, nsCStr) \
const nsCString& promiseFlatCStr = PromiseFlatCString(nsCStr); \
mozilla::AutoProfilerLabel \
PROFILER_RAII(label, promiseFlatCStr.get(), __LINE__, \
js::ProfileEntry::Category::category); \
// Similar to AUTO_PROFILER_LABEL_DYNAMIC_CSTR, but takes an nsString that is
// is lossily converted to an ASCII string.
//
// Note: The use of the Maybe<>s ensures the scopes for the converted dynamic
// string and the AutoProfilerLabel are appropriate, while also not incurring
// the runtime cost of the string conversion unless the profiler is active.
// Therefore, unlike other AUTO_PROFILER_LABEL* macros, this one doesn't
// push/pop a label when the profiler is inactive.
#define AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(label, category, nsStr) \
mozilla::Maybe<NS_LossyConvertUTF16toASCII> asciiStr; \
mozilla::Maybe<AutoProfilerLabel> raiiObjectLossy; \
if (profiler_is_active()) { \
asciiStr.emplace(nsStr); \
raiiObjectLossy.emplace(label, asciiStr->get(), __LINE__, \
js::ProfileEntry::Category::category); \
}
// Insert a marker in the profile timeline. This is useful to delimit something
// important happening such as the first paint. Unlike labels, which are only

View File

@ -494,9 +494,9 @@ TEST(GeckoProfiler, Markers)
okstr1[kMax - 1] = '\0';
okstr2[kMax - 1] = '\0';
longstr[kMax] = '\0';
AUTO_PROFILER_LABEL_DYNAMIC("", CSS, okstr1.get());
AUTO_PROFILER_LABEL_DYNAMIC("okstr2", CSS, okstr2.get());
AUTO_PROFILER_LABEL_DYNAMIC("", CSS, longstr.get());
AUTO_PROFILER_LABEL_DYNAMIC_CSTR("", CSS, okstr1.get());
AUTO_PROFILER_LABEL_DYNAMIC_CSTR("okstr2", CSS, okstr2.get());
AUTO_PROFILER_LABEL_DYNAMIC_CSTR("", CSS, longstr.get());
// Sleep briefly to ensure a sample is taken and the pending markers are
// processed.
@ -697,7 +697,11 @@ TEST(GeckoProfiler, PseudoStack)
UniqueFreePtr<char> dynamic(strdup("dynamic"));
{
AUTO_PROFILER_LABEL_DYNAMIC("A::C", JS, dynamic.get());
AUTO_PROFILER_LABEL_DYNAMIC_CSTR("A::C", JS, dynamic.get());
AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING(
"A::C2", JS, nsDependentCString(dynamic.get()));
AUTO_PROFILER_LABEL_DYNAMIC_LOSSY_NSSTRING(
"A::C3", JS, NS_ConvertUTF8toUTF16(dynamic.get()));
profiler_start(PROFILER_DEFAULT_ENTRIES, PROFILER_DEFAULT_INTERVAL,
features, filters, MOZ_ARRAY_LENGTH(filters));

View File

@ -288,8 +288,8 @@ NS_IMETHODIMP nsObserverService::NotifyObservers(nsISupports* aSubject,
mozilla::TimeStamp start = TimeStamp::Now();
AUTO_PROFILER_LABEL_DYNAMIC("nsObserverService::NotifyObservers", OTHER,
aTopic);
AUTO_PROFILER_LABEL_DYNAMIC_CSTR(
"nsObserverService::NotifyObservers", OTHER, aTopic);
nsObserverList* observerList = mObserverTopicTable.GetEntry(aTopic);
if (observerList) {