mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1576555 - Don't store temporary ProfilerMarkerPayloads in UniquePtrs - r=gregtatum 

Since payloads are not kept alive long after they have been serialized, we can
just create them on the stack and pass a reference to their base (or pointer,
`nullptr` representing "no payloads") to `profiler_add_marker()`.

Differential Revision: https://phabricator.services.mozilla.com/D43430

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Gerald Squelart 2019-09-18 01:21:02 +00:00
Родитель 666a5caa9d
Коммит 02d2886883
7 изменённых файлов: 108 добавлений и 89 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

53
mozglue/baseprofiler/core/platform.cpp
Просмотреть файл

@ -3198,17 +3198,19 @@ void ProfilerBacktraceDestructor::operator()(ProfilerBacktrace* aBacktrace) {
delete aBacktrace;
}
static void racy_profiler_add_marker(
const char* aMarkerName, ProfilingCategoryPair aCategoryPair,
UniquePtr<ProfilerMarkerPayload> aPayload) {
static void racy_profiler_add_marker(const char* aMarkerName,
ProfilingCategoryPair aCategoryPair,
const ProfilerMarkerPayload* aPayload) {
MOZ_RELEASE_ASSERT(CorePS::Exists());
// We don't assert that RacyFeatures::IsActiveWithoutPrivacy() or
// RacyRegisteredThread::IsBeingProfiled() is true here, because it's
// possible that the result has changed since we tested it in the caller.
//
// Because of this imprecision it's possible to miss a marker or record one
// we shouldn't. Either way is not a big deal.
// This function is hot enough that we use RacyFeatures, not ActivePS.
if (!RacyFeatures::IsActiveWithoutPrivacy()) {
return;
}
// Note that it's possible that the above test would change again before we
// actually record the marker. Because of this imprecision it's possible to
// miss a marker or record one we shouldn't. Either way is not a big deal.
RacyRegisteredThread* racyRegisteredThread =
TLSRegisteredThread::RacyRegisteredThread();
@ -3228,27 +3230,20 @@ static void racy_profiler_add_marker(
void profiler_add_marker(const char* aMarkerName,
ProfilingCategoryPair aCategoryPair,
UniquePtr<ProfilerMarkerPayload> aPayload) {
MOZ_RELEASE_ASSERT(CorePS::Exists());
// This function is hot enough that we use RacyFeatures, not ActivePS.
if (!RacyFeatures::IsActiveWithoutPrivacy()) {
return;
}
racy_profiler_add_marker(aMarkerName, aCategoryPair, std::move(aPayload));
const ProfilerMarkerPayload& aPayload) {
racy_profiler_add_marker(aMarkerName, aCategoryPair, &aPayload);
}
void profiler_add_marker(const char* aMarkerName,
ProfilingCategoryPair aCategoryPair) {
profiler_add_marker(aMarkerName, aCategoryPair, nullptr);
racy_profiler_add_marker(aMarkerName, aCategoryPair, nullptr);
}
// This is a simplified version of profiler_add_marker that can be easily passed
// into the JS engine.
void profiler_add_js_marker(const char* aMarkerName) {
AUTO_PROFILER_STATS(base_add_marker);
profiler_add_marker(aMarkerName, ProfilingCategoryPair::JS, nullptr);
profiler_add_marker(aMarkerName, ProfilingCategoryPair::JS);
}
// This logic needs to add a marker for a different thread, so we actually need
@ -3306,9 +3301,9 @@ void profiler_tracing(const char* aCategoryString, const char* aMarkerName,
}
AUTO_PROFILER_STATS(base_add_marker_with_TracingMarkerPayload);
auto payload = MakeUnique<TracingMarkerPayload>(
aCategoryString, aKind, aDocShellId, aDocShellHistoryId);
racy_profiler_add_marker(aMarkerName, aCategoryPair, std::move(payload));
profiler_add_marker(aMarkerName, aCategoryPair,
TracingMarkerPayload(aCategoryString, aKind, aDocShellId,
aDocShellHistoryId));
}
void profiler_tracing(const char* aCategoryString, const char* aMarkerName,
@ -3326,10 +3321,10 @@ void profiler_tracing(const char* aCategoryString, const char* aMarkerName,
}
AUTO_PROFILER_STATS(base_add_marker_with_TracingMarkerPayload);
auto payload =
MakeUnique<TracingMarkerPayload>(aCategoryString, aKind, aDocShellId,
aDocShellHistoryId, std::move(aCause));
racy_profiler_add_marker(aMarkerName, aCategoryPair, std::move(payload));
profiler_add_marker(
aMarkerName, aCategoryPair,
TracingMarkerPayload(aCategoryString, aKind, aDocShellId,
aDocShellHistoryId, std::move(aCause)));
}
void profiler_add_text_marker(const char* aMarkerName, const std::string& aText,
@ -3342,8 +3337,8 @@ void profiler_add_text_marker(const char* aMarkerName, const std::string& aText,
AUTO_PROFILER_STATS(base_add_marker_with_TextMarkerPayload);
profiler_add_marker(
aMarkerName, aCategoryPair,
MakeUnique<TextMarkerPayload>(aText, aStartTime, aEndTime, aDocShellId,
aDocShellHistoryId, std::move(aCause)));
TextMarkerPayload(aText, aStartTime, aEndTime, aDocShellId,
aDocShellHistoryId, std::move(aCause)));
}
// NOTE: aCollector's methods will be called while the target thread is paused.

5
mozglue/baseprofiler/public/BaseProfiler.h
Просмотреть файл

@ -712,12 +712,13 @@ MFBT_API void profiler_add_marker(const char* aMarkerName,
::mozilla::baseprofiler::profiler_add_marker( \
markerName, \
::mozilla::baseprofiler::ProfilingCategoryPair::categoryPair, \
::mozilla::MakeUnique<PayloadType> parenthesizedPayloadArgs); \
PayloadType parenthesizedPayloadArgs); \
} while (false)
MFBT_API void profiler_add_marker(const char* aMarkerName,
ProfilingCategoryPair aCategoryPair,
UniquePtr<ProfilerMarkerPayload> aPayload);
const ProfilerMarkerPayload& aPayload);
MFBT_API void profiler_add_js_marker(const char* aMarkerName);
// Insert a marker in the profile timeline for a specified thread.

15
mozglue/baseprofiler/public/BaseProfilerMarkerPayload.h
Просмотреть файл

@ -333,6 +333,21 @@ class LogMarkerPayload : public ProfilerMarkerPayload {
// Serialize a pointed-at ProfilerMarkerPayload, may be null when there are no
// payloads.
template <>
struct BlocksRingBuffer::Serializer<
const baseprofiler::ProfilerMarkerPayload*> {
static Length Bytes(const baseprofiler::ProfilerMarkerPayload* aPayload) {
return baseprofiler::ProfilerMarkerPayload::TagAndSerializationBytes(
aPayload);
}
static void Write(EntryWriter& aEW,
const baseprofiler::ProfilerMarkerPayload* aPayload) {
baseprofiler::ProfilerMarkerPayload::TagAndSerialize(aPayload, aEW);
}
};
// Serialize a pointed-at ProfilerMarkerPayload, may be null for no payloads.
template <>
struct BlocksRingBuffer::Serializer<
UniquePtr<baseprofiler::ProfilerMarkerPayload>> {
static Length Bytes(

33
mozglue/tests/TestBaseProfiler.cpp
Просмотреть файл

@ -1475,10 +1475,9 @@ void TestProfilerMarkerSerialization() {
constexpr int data = 42;
{
UniquePtr<baseprofiler::ProfilerMarkerPayload> testPayload{
new BaseTestMarkerPayload(data)};
rb.PutObject(testPayload);
BaseTestMarkerPayload payload(data);
rb.PutObject(
static_cast<const baseprofiler::ProfilerMarkerPayload*>(&payload));
}
int read = 0;
@ -1644,8 +1643,8 @@ void TestProfiler() {
// Just making sure all payloads know how to (de)serialize and stream.
baseprofiler::profiler_add_marker(
"TracingMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER,
MakeUnique<baseprofiler::TracingMarkerPayload>(
"category", baseprofiler::TRACING_EVENT));
baseprofiler::TracingMarkerPayload("category",
baseprofiler::TRACING_EVENT));
auto cause =
# if defined(__linux__) || defined(__ANDROID__)
@ -1656,37 +1655,37 @@ void TestProfiler() {
# endif
baseprofiler::profiler_add_marker(
"FileIOMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER,
MakeUnique<baseprofiler::FileIOMarkerPayload>(
baseprofiler::FileIOMarkerPayload(
"operation", "source", "filename", TimeStamp::NowUnfuzzed(),
TimeStamp::NowUnfuzzed(), std::move(cause)));
baseprofiler::profiler_add_marker(
"UserTimingMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER,
MakeUnique<baseprofiler::UserTimingMarkerPayload>(
"name", TimeStamp::NowUnfuzzed(), Nothing{}, Nothing{}));
baseprofiler::UserTimingMarkerPayload("name", TimeStamp::NowUnfuzzed(),
Nothing{}, Nothing{}));
baseprofiler::profiler_add_marker(
"HangMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER,
MakeUnique<baseprofiler::HangMarkerPayload>(TimeStamp::NowUnfuzzed(),
TimeStamp::NowUnfuzzed()));
baseprofiler::HangMarkerPayload(TimeStamp::NowUnfuzzed(),
TimeStamp::NowUnfuzzed()));
baseprofiler::profiler_add_marker(
"LongTaskMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER,
MakeUnique<baseprofiler::LongTaskMarkerPayload>(
TimeStamp::NowUnfuzzed(), TimeStamp::NowUnfuzzed()));
baseprofiler::LongTaskMarkerPayload(TimeStamp::NowUnfuzzed(),
TimeStamp::NowUnfuzzed()));
{
std::string s = "text payload";
baseprofiler::profiler_add_marker(
"TextMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER,
MakeUnique<baseprofiler::TextMarkerPayload>(
s, TimeStamp::NowUnfuzzed(), TimeStamp::NowUnfuzzed()));
baseprofiler::TextMarkerPayload(s, TimeStamp::NowUnfuzzed(),
TimeStamp::NowUnfuzzed()));
}
baseprofiler::profiler_add_marker(
"LogMarkerPayload", baseprofiler::ProfilingCategoryPair::OTHER,
MakeUnique<baseprofiler::LogMarkerPayload>("module", "text",
TimeStamp::NowUnfuzzed()));
baseprofiler::LogMarkerPayload("module", "text",
TimeStamp::NowUnfuzzed()));
printf("Sleep 1s...\n");
{

59
tools/profiler/core/platform.cpp
Просмотреть файл

@ -4067,17 +4067,19 @@ void ProfilerBacktraceDestructor::operator()(ProfilerBacktrace* aBacktrace) {
delete aBacktrace;
}
static void racy_profiler_add_marker(
const char* aMarkerName, JS::ProfilingCategoryPair aCategoryPair,
UniquePtr<ProfilerMarkerPayload> aPayload) {
static void racy_profiler_add_marker(const char* aMarkerName,
JS::ProfilingCategoryPair aCategoryPair,
const ProfilerMarkerPayload* aPayload) {
MOZ_RELEASE_ASSERT(CorePS::Exists());
// We don't assert that RacyFeatures::IsActiveWithoutPrivacy() or
// RacyRegisteredThread::IsBeingProfiled() is true here, because it's
// possible that the result has changed since we tested it in the caller.
//
// Because of this imprecision it's possible to miss a marker or record one
// we shouldn't. Either way is not a big deal.
// This function is hot enough that we use RacyFeatures, not ActivePS.
if (!RacyFeatures::IsActiveWithoutPrivacy()) {
return;
}
// Note that it's possible that the above test would change again before we
// actually record the marker. Because of this imprecision it's possible to
// miss a marker or record one we shouldn't. Either way is not a big deal.
RacyRegisteredThread* racyRegisteredThread =
TLSRegisteredThread::RacyRegisteredThread();
@ -4097,35 +4099,28 @@ static void racy_profiler_add_marker(
void profiler_add_marker(const char* aMarkerName,
JS::ProfilingCategoryPair aCategoryPair,
UniquePtr<ProfilerMarkerPayload> aPayload) {
MOZ_RELEASE_ASSERT(CorePS::Exists());
// This function is hot enough that we use RacyFeatures, not ActivePS.
if (!RacyFeatures::IsActiveWithoutPrivacy()) {
return;
}
racy_profiler_add_marker(aMarkerName, aCategoryPair, std::move(aPayload));
const ProfilerMarkerPayload& aPayload) {
racy_profiler_add_marker(aMarkerName, aCategoryPair, &aPayload);
}
void profiler_add_marker(const char* aMarkerName,
JS::ProfilingCategoryPair aCategoryPair) {
profiler_add_marker(aMarkerName, aCategoryPair, nullptr);
racy_profiler_add_marker(aMarkerName, aCategoryPair, nullptr);
}
// This is a simplified version of profiler_add_marker that can be easily passed
// into the JS engine.
void profiler_add_js_marker(const char* aMarkerName) {
AUTO_PROFILER_STATS(add_marker);
profiler_add_marker(aMarkerName, JS::ProfilingCategoryPair::JS, nullptr);
profiler_add_marker(aMarkerName, JS::ProfilingCategoryPair::JS);
}
void profiler_add_js_allocation_marker(JS::RecordAllocationInfo&& info) {
AUTO_PROFILER_STATS(add_marker_with_JsAllocationMarkerPayload);
profiler_add_marker(
"JS allocation", JS::ProfilingCategoryPair::JS,
MakeUnique<JsAllocationMarkerPayload>(TimeStamp::Now(), std::move(info),
profiler_get_backtrace()));
JsAllocationMarkerPayload(TimeStamp::Now(), std::move(info),
profiler_get_backtrace()));
}
void profiler_add_network_marker(
@ -4153,7 +4148,7 @@ void profiler_add_network_marker(
AUTO_PROFILER_STATS(add_marker_with_NetworkMarkerPayload);
profiler_add_marker(
name, JS::ProfilingCategoryPair::NETWORK,
MakeUnique<NetworkMarkerPayload>(
NetworkMarkerPayload(
static_cast<int64_t>(aChannelId), PromiseFlatCString(spec).get(),
aType, aStart, aEnd, aPriority, aCount, aCacheDisposition, aTimings,
PromiseFlatCString(redirect_spec).get(), std::move(aSource)));
@ -4211,9 +4206,9 @@ void profiler_tracing(const char* aCategoryString, const char* aMarkerName,
}
AUTO_PROFILER_STATS(add_marker_with_TracingMarkerPayload);
auto payload = MakeUnique<TracingMarkerPayload>(
aCategoryString, aKind, aDocShellId, aDocShellHistoryId);
racy_profiler_add_marker(aMarkerName, aCategoryPair, std::move(payload));
profiler_add_marker(aMarkerName, aCategoryPair,
TracingMarkerPayload(aCategoryString, aKind, aDocShellId,
aDocShellHistoryId));
}
void profiler_tracing(const char* aCategoryString, const char* aMarkerName,
@ -4231,10 +4226,10 @@ void profiler_tracing(const char* aCategoryString, const char* aMarkerName,
}
AUTO_PROFILER_STATS(add_marker_with_TracingMarkerPayload);
auto payload =
MakeUnique<TracingMarkerPayload>(aCategoryString, aKind, aDocShellId,
aDocShellHistoryId, std::move(aCause));
racy_profiler_add_marker(aMarkerName, aCategoryPair, std::move(payload));
profiler_add_marker(
aMarkerName, aCategoryPair,
TracingMarkerPayload(aCategoryString, aKind, aDocShellId,
aDocShellHistoryId, std::move(aCause)));
}
void profiler_add_text_marker(
@ -4247,8 +4242,8 @@ void profiler_add_text_marker(
AUTO_PROFILER_STATS(add_marker_with_TextMarkerPayload);
profiler_add_marker(
aMarkerName, aCategoryPair,
MakeUnique<TextMarkerPayload>(aText, aStartTime, aEndTime, aDocShellId,
aDocShellHistoryId, std::move(aCause)));
TextMarkerPayload(aText, aStartTime, aEndTime, aDocShellId,
aDocShellHistoryId, std::move(aCause)));
}
void profiler_set_js_context(JSContext* aCx) {

17
tools/profiler/public/GeckoProfiler.h
Просмотреть файл

@ -691,18 +691,19 @@ void profiler_add_marker(const char* aMarkerName,
// the argument list used to construct that `PayloadType`. E.g.:
// `PROFILER_ADD_MARKER_WITH_PAYLOAD("Load", DOM, TextMarkerPayload,
// ("text", start, end, ds, dsh))`
# define PROFILER_ADD_MARKER_WITH_PAYLOAD( \
markerName, categoryPair, PayloadType, parenthesizedPayloadArgs) \
do { \
AUTO_PROFILER_STATS(add_marker_with_##PayloadType); \
::profiler_add_marker( \
markerName, ::JS::ProfilingCategoryPair::categoryPair, \
::mozilla::MakeUnique<PayloadType> parenthesizedPayloadArgs); \
# define PROFILER_ADD_MARKER_WITH_PAYLOAD( \
markerName, categoryPair, PayloadType, parenthesizedPayloadArgs) \
do { \
AUTO_PROFILER_STATS(add_marker_with_##PayloadType); \
::profiler_add_marker(markerName, \
::JS::ProfilingCategoryPair::categoryPair, \
PayloadType parenthesizedPayloadArgs); \
} while (false)
void profiler_add_marker(const char* aMarkerName,
JS::ProfilingCategoryPair aCategoryPair,
mozilla::UniquePtr<ProfilerMarkerPayload> aPayload);
const ProfilerMarkerPayload& aPayload);
void profiler_add_js_marker(const char* aMarkerName);
void profiler_add_js_allocation_marker(JS::RecordAllocationInfo&& info);

15
tools/profiler/public/ProfilerMarkerPayload.h
Просмотреть файл

@ -651,7 +651,7 @@ class LogMarkerPayload : public ProfilerMarkerPayload {
class JsAllocationMarkerPayload : public ProfilerMarkerPayload {
public:
JsAllocationMarkerPayload(const mozilla::TimeStamp& aStartTime,
const JS::RecordAllocationInfo& aInfo,
JS::RecordAllocationInfo&& aInfo,
UniqueProfilerBacktrace aStack)
: ProfilerMarkerPayload(aStartTime, aStartTime, mozilla::Nothing(),
mozilla::Nothing(), std::move(aStack)),
@ -699,6 +699,19 @@ class JsAllocationMarkerPayload : public ProfilerMarkerPayload {
namespace mozilla {
// Serialize a pointed-at ProfilerMarkerPayload, may be null when there are no
// payloads.
template <>
struct BlocksRingBuffer::Serializer<const ProfilerMarkerPayload*> {
static Length Bytes(const ProfilerMarkerPayload* aPayload) {
return ProfilerMarkerPayload::TagAndSerializationBytes(aPayload);
}
static void Write(EntryWriter& aEW, const ProfilerMarkerPayload* aPayload) {
ProfilerMarkerPayload::TagAndSerialize(aPayload, aEW);
}
};
// Serialize a pointed-at ProfilerMarkerPayload, may be null when there are no
// payloads.
template <>