Closed
Bug 1337189
Opened 7 years ago
Closed 7 years ago
Merge Sampler.cpp into platform.cpp
Categories
(Core :: Gecko Profiler, defect)
Core
Gecko Profiler
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(28 files, 4 obsolete files)
10.58 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
3.27 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
2.12 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
10.50 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
9.67 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
1.46 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
11.78 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
5.73 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
5.05 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
3.47 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
7.05 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
7.58 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
1.79 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
9.87 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
16.63 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
11.73 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
19.33 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
3.00 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
4.83 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
57.88 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
8.39 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
4.33 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
5.40 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
5.73 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
5.35 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
27.11 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
11.45 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
1.37 KB,
patch
|
schien
:
review+
|
Details | Diff | Splinter Review |
Sampler is a class that only ever has zero or one instances, depending on whether the profiler is running. All of its state is effectively global; even when it's not instantiated, profiler_*() functions often need to do something stateful anyway. (See the sIs* variables in platform.cpp for an unpleasant and illustrative example.) platform.cpp already has a number of global variables which are accessed via the profiler_*() functions. Merging Sampler into platform.cpp will remove an unhelpful layer of separation and put all the relevant code in one file. Sampler's fields will become global variables in platform.cpp. These changes will make the code simpler, more uniform, and more readable. It will also make it much easier to see which state is accessed from multiple threads and thus needs synchronization.
Assignee | ||
Comment 1•7 years ago
|
||
This function indictes when profiler_init() is called, not profiler_start(). Renaming it as sInitTime makes that clearer.
Attachment #8834197 -
Flags: review?(mstange)
Assignee | ||
Comment 2•7 years ago
|
||
This patch adds NS_IsMainThread() assertions to all main-thread-only profiler_*() functions that currently lack them, and adds comments to those that run on multiple threads. As a result, it's now clear for every profiler_*() function which threads it runs on.
Attachment #8834201 -
Flags: review?(mstange)
Assignee | ||
Comment 3•7 years ago
|
||
Attachment #8834203 -
Flags: review?(mstange)
Assignee | ||
Updated•7 years ago
|
Attachment #8834201 -
Attachment is obsolete: true
Attachment #8834201 -
Flags: review?(mstange)
Assignee | ||
Comment 4•7 years ago
|
||
All it does is set sLastTracerEvent, which is never read.
Attachment #8834210 -
Flags: review?(mstange)
Assignee | ||
Comment 5•7 years ago
|
||
It doesn't need to be visible outside the profiler.
Attachment #8834211 -
Flags: review?(mstange)
Assignee | ||
Comment 6•7 years ago
|
||
I am planning to merge Sampler into platform.cpp, so Sampler.cpp will disappear. This change will make that easier, because things that temporarily need to be visible in both files won't need to be declared in a header.
Attachment #8834212 -
Flags: review?(mstange)
Assignee | ||
Comment 7•7 years ago
|
||
They don't particularly belong there, and Sampler will be going away eventually anyway.
Attachment #8834213 -
Flags: review?(mstange)
Assignee | ||
Comment 8•7 years ago
|
||
This is a good example of code that's in Sampler for no particular reason.
Attachment #8834215 -
Flags: review?(mstange)
Assignee | ||
Comment 9•7 years ago
|
||
Attachment #8834220 -
Flags: review?(mstange)
Assignee | ||
Comment 10•7 years ago
|
||
Whoops, I completely misunderstood how this variable is used in my previous patch.
Attachment #8834240 -
Flags: review?(mstange)
Assignee | ||
Updated•7 years ago
|
Attachment #8834197 -
Attachment is obsolete: true
Attachment #8834197 -
Flags: review?(mstange)
Assignee | ||
Comment 11•7 years ago
|
||
Doing so made it clearer that gThreadNameFilters was being accessed from multiple threads without synchronization, so I added a Mutex for it.
Attachment #8834250 -
Flags: review?(mstange)
Assignee | ||
Comment 12•7 years ago
|
||
Attachment #8834251 -
Flags: review?(mstange)
Assignee | ||
Comment 13•7 years ago
|
||
Attachment #8834258 -
Flags: review?(mstange)
Assignee | ||
Comment 14•7 years ago
|
||
Attachment #8834260 -
Flags: review?(mstange)
Assignee | ||
Comment 15•7 years ago
|
||
platform-*.cc and platform.cpp belong together conceptually, and combining them into a single compilation unit makes it easier to share things and avoids the need for some declarations in headers. The patch also removes old_sigsave_signal_handler_ which is a long-dead field that clang now detects and complains about.
Attachment #8834268 -
Flags: review?(mstange)
Assignee | ||
Comment 16•7 years ago
|
||
Attachment #8834271 -
Flags: review?(mstange)
Assignee | ||
Updated•7 years ago
|
Attachment #8834213 -
Attachment is obsolete: true
Attachment #8834213 -
Flags: review?(mstange)
Assignee | ||
Comment 17•7 years ago
|
||
Attachment #8834275 -
Flags: review?(mstange)
Assignee | ||
Updated•7 years ago
|
Attachment #8834211 -
Attachment is obsolete: true
Attachment #8834211 -
Flags: review?(mstange)
Assignee | ||
Comment 18•7 years ago
|
||
Apologies for the patch shuffling; while writing later patches I realized I could improve some of the earlier ones. I have more to do here, but these patches give you a good idea of the direction I'm heading in.
Updated•7 years ago
|
Attachment #8834240 -
Flags: review?(mstange) → review+
Comment 19•7 years ago
|
||
Comment on attachment 8834203 [details] [diff] [review] (part 2) - Add more threadedness assertions and comments to the profiler Review of attachment 8834203 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/profiler/core/platform.cpp @@ +1078,5 @@ > profiler_time(const mozilla::TimeStamp& aTime) > { > + // This function currently only used on the main thread, but that restriction > + // (and this assertion) could be removed trivially because it doesn't touch > + // data that requires locking. It touches sStartTime, so I don't think this comment is correct.
Attachment #8834203 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8834210 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8834268 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8834212 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8834215 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8834220 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8834250 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8834251 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8834258 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8834260 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8834271 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8834275 -
Flags: review?(mstange) → review+
Comment 20•7 years ago
|
||
Do you have plans to reduce the number of global variables and locks once this is done and the access + usage patterns have become clearer? For example we should be able to group some of these fields into structs, and then maybe use a single mutex for all accesses of the same group / struct. For example the init params (set by profiler_start, read by any thread) seem like they should be such a group.
Assignee | ||
Comment 21•7 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #20) > Do you have plans to reduce the number of global variables and locks once > this is done and the access + usage patterns have become clearer? Yes! Not sure yet what it will look like, but I definitely want to do something like that.
Assignee | ||
Comment 22•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b2aa39f93cbcf6b69f4ee2dafc505010850daa87 Bug 1337189 (part 1) - Add a comment about sStartTime. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/499daf10f2ff638df8400e56cbd217d61a47be8c Bug 1337189 (part 2) - Add more threadedness assertions and comments to the profiler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/6f55b285bad3f6f84035bb392fde332f76fd6f4c Bug 1337189 (part 3) - Remove profiler_responsiveness. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/e6f643603520c375fdcea9e8f0273d0862e7b783 Bug 1337189 (part 4) - #include platform-*.cc from platform.cpp. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/9fb286937e67a44ae1de84157dabe1f50b0d2562 Bug 1337189 (part 5) - #include Sampler.cpp from platform.cpp. r=mstange.
Assignee | ||
Updated•7 years ago
|
Keywords: leave-open
Summary: Merge Sampler into profiler.cpp → Merge Sampler into platform.cpp
Assignee | ||
Comment 23•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/85905c454c0553c12ef3db1f12415229b9c980c5 Bug 1337189 (part 6) - Move sRegisteredThreads{,Mutex} out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/81c0116b6037778653111d4052410722ed434355 Bug 1337189 (part 6b) - Move declarations of gSampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/3cbe13875035b36735dcac2489abfcaa95dc9a69 Bug 1337189 (part 7) - Move Startup(), Shutdown(), RegisterCurrentThread(), UnregisterCurrentThread() out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/572e9915387cd18c0f5198dfbb74b435a550d1ee Bug 1337189 (part 8) - Move mGatherer out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/4920597f7463dca5ae5c0e4eea050ceb9d9c716a Bug 1337189 (part 9) - Move mThreadNameFilters and mFeatures out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/df5a1de42f04e5d4e5e551d28d049911a4035f2d Bug 1337189 (part 10) - Move entrySize_ out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/5b47c959f0c6a49786cecf930f869436939f0454 Bug 1337189 (part 11) - Move the Linux-specific fields out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/46234bd1f1d778eb2c58b859673644a7890f0ac7 Bug 1337189 (part 12) - Move s{,Last}FrameNumber from Sampler.cpp to platform.cpp. r=mstange.
Assignee | ||
Comment 24•7 years ago
|
||
Attachment #8835336 -
Flags: review?(mstange)
Assignee | ||
Comment 25•7 years ago
|
||
Attachment #8835337 -
Flags: review?(mstange)
Assignee | ||
Comment 26•7 years ago
|
||
Attachment #8835338 -
Flags: review?(mstange)
Assignee | ||
Comment 27•7 years ago
|
||
Two of them (mAddMainThreadIO and mPrivacyMode) are now local variables within profiler_start(), which is nice. This change also lets us get rid of four sIs* variables.
Attachment #8835339 -
Flags: review?(mstange)
Assignee | ||
Comment 28•7 years ago
|
||
This means PseudoStack::flushSamplerOnJSShutdown() should now work when called off-main-thread.
Attachment #8835340 -
Flags: review?(mstange)
Assignee | ||
Comment 29•7 years ago
|
||
The patch also removes NewSyncProfile() by inlining it at its single call site.
Attachment #8835341 -
Flags: review?(mstange)
Assignee | ||
Comment 30•7 years ago
|
||
The patch also removes InplaceTick(), which wasn't serving any useful purpose.
Attachment #8835342 -
Flags: review?(mstange)
Assignee | ||
Comment 31•7 years ago
|
||
The patch also renames them as PlatformStart() and PlatformStop() to make it clearer they are platform-specific.
Attachment #8835344 -
Flags: review?(mstange)
Assignee | ||
Comment 32•7 years ago
|
||
Attachment #8835345 -
Flags: review?(mstange)
Assignee | ||
Comment 33•7 years ago
|
||
Attachment #8835346 -
Flags: review?(mstange)
Assignee | ||
Comment 34•7 years ago
|
||
Attachment #8835347 -
Flags: review?(mstange)
Assignee | ||
Comment 35•7 years ago
|
||
The new name is MaybeSetProfile(), which better reflects what it does.
Attachment #8835348 -
Flags: review?(mstange)
Assignee | ||
Comment 36•7 years ago
|
||
Attachment #8835349 -
Flags: review?(mstange)
Assignee | ||
Comment 37•7 years ago
|
||
Sampler's constructor and destructor are inlined into profiler_{start,stop}(), respectively. A trivial Sampler class is left behind (now in platform.cpp) because there are still numerous gSampler null checks, which I will remove in a follow-up bug. PseudoStack::flushSamplerOnShutdown() moves into platform.cpp. MAXPATHLEN is unused and is deleted.
Attachment #8835350 -
Flags: review?(mstange)
Assignee | ||
Comment 38•7 years ago
|
||
After those 26 patches Sampler.cpp is gone but the Sampler class still exists, albeit trivially. That is enough patches for this bug, I'll remove Sampler and do other improvements in follow-up bugs.
Summary: Merge Sampler into platform.cpp → Merge Sampler.cpp into platform.cpp
Comment 39•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b2aa39f93cbc https://hg.mozilla.org/mozilla-central/rev/499daf10f2ff https://hg.mozilla.org/mozilla-central/rev/6f55b285bad3 https://hg.mozilla.org/mozilla-central/rev/e6f643603520 https://hg.mozilla.org/mozilla-central/rev/9fb286937e67 https://hg.mozilla.org/mozilla-central/rev/85905c454c05 https://hg.mozilla.org/mozilla-central/rev/81c0116b6037 https://hg.mozilla.org/mozilla-central/rev/3cbe13875035 https://hg.mozilla.org/mozilla-central/rev/572e9915387c https://hg.mozilla.org/mozilla-central/rev/4920597f7463 https://hg.mozilla.org/mozilla-central/rev/df5a1de42f04 https://hg.mozilla.org/mozilla-central/rev/5b47c959f0c6 https://hg.mozilla.org/mozilla-central/rev/46234bd1f1d7
Updated•7 years ago
|
Attachment #8835336 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8835337 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8835338 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8835339 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8835340 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8835341 -
Flags: review?(mstange) → review+
Comment 40•7 years ago
|
||
Comment on attachment 8835342 [details] [diff] [review] (part 19) - Move Tick() and GetThreadHandle() out of sampler Review of attachment 8835342 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/profiler/core/platform.cpp @@ +564,5 @@ > + StackWalkCallback(/* frameNum */ 0, aSample->pc, aSample->sp, &nativeStack); > + > + uint32_t maxFrames = uint32_t(nativeStack.size - nativeStack.count); > + > + // Win X64 doesn't support disabling frame pointers emission so we need to Might want to fix that comment up to say "frame pointer omission" while you're here.
Attachment #8835342 -
Flags: review?(mstange) → review+
Comment 41•7 years ago
|
||
Comment on attachment 8835344 [details] [diff] [review] (part 20) - Move Start() and Stop() out of Sampler Review of attachment 8835344 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/profiler/core/platform.cpp @@ +1535,5 @@ > } > > +// Platform-specific start/stop actions. > +static void PlatformStart(); > +static void PlatformStop(); Is it intentional that the declaration has "static" but the definitions don't?
Attachment #8835344 -
Flags: review?(mstange) → review+
Comment 42•7 years ago
|
||
Comment on attachment 8835345 [details] [diff] [review] (part 21) - Remove Sampler::SizeOfIncludingThis() Review of attachment 8835345 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/profiler/core/platform.cpp @@ +1149,5 @@ > + // Measurement of the following things may be added later if DMD finds it > + // is worthwhile: > + // - memory pointed to by the elements within gBuffer > + // - mThreadNameFilters > + // - mFeatures These are called gThreadNameFilters and gFeatures now.
Attachment #8835345 -
Flags: review?(mstange) → review+
Comment 43•7 years ago
|
||
Comment on attachment 8835346 [details] [diff] [review] (part 22) - Move CanNotifyObservers() out of Sampler Review of attachment 8835346 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/profiler/core/platform.cpp @@ +199,5 @@ > + > +#ifdef MOZ_WIDGET_GONK > + // We use profile.sh on b2g to manually select threads and options per > + // process. > + return false; Oh, this stuff can go for sure. We're not using profile.sh anymore.
Attachment #8835346 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8835347 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8835348 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8835349 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8835350 -
Flags: review?(mstange) → review+
Assignee | ||
Comment 44•7 years ago
|
||
> > + // Win X64 doesn't support disabling frame pointers emission so we need to > > Might want to fix that comment up to say "frame pointer omission" while > you're here. LOL > > +// Platform-specific start/stop actions. > > +static void PlatformStart(); > > +static void PlatformStop(); > > Is it intentional that the declaration has "static" but the definitions > don't? Not particularly. I will add "static" to the definitions.
Assignee | ||
Comment 45•7 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #44) > > > + // Win X64 doesn't support disabling frame pointers emission so we need to > > > > Might want to fix that comment up to say "frame pointer omission" while > > you're here. > > LOL Actually, that comment is awful -- the triple negative makes it hard to read. Here's my revised version: // Win64 always omits frame pointers so for it we fall back to using the // slower MozStackWalk().
Comment 46•7 years ago
|
||
Ok, that sounds much better, thanks.
Assignee | ||
Comment 47•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc0ad9814a6c6475c6a2b30cc6296e3a73730a4e Bug 1337189 (part 13) - Move interval_ out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/8694332bf02aac93abb471b334e0b51721cd3628 Bug 1337189 (part 14) - Move active_ and paused_ out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/9d9c499755393bb02be0c0f264bfe8ec3addf9fd Bug 1337189 (part 15) - Move mBuffer out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/4a894b2316f0a6910aeb6243060497ef277ed847 Bug 1337189 (part 16) - Move all the bools out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/99c10f2d0d76fe91dacb340cf3b3ef0ac50976b5 Bug 1337189 (part 17) - Remove Sampler::FlushOnJSShutdown(). r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/6a94811a6ef0d9eff21ae877e28cbf9d96bf1e31 Bug 1337189 (part 18) - Remove Sampler::GetBacktrace(). r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/778675533fb9c6ab7f33a32c94fd20baf5436db9 Bug 1337189 (part 19) - Move Tick() and GetThreadHandle() out of sampler. r=mstange.
Assignee | ||
Comment 48•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4c677b0b92faa62a6694b4fb7d44f44371923b2 Bug 1337189 (part 20) - Move Start() and Stop() out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/63172ee12b036c5201d61a48bc242767e1e069b3 Bug 1337189 (part 21) - Remove Sampler::SizeOfIncludingThis(). r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/6c209f8f314b33337c9fee50bcb7bb87236fab6b Bug 1337189 (part 22) - Move CanNotifyObservers() out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/3b603e66e2765ea37409b72595bf16f27c3d91a9 Bug 1337189 (part 23) - Move PlatformData out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/5805116bb2e09ba42b7c385fc867e3ac31575730 Bug 1337189 (part 24) - Move RegisterThread() out of Sampler and rename it. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/aa0f4dd3ce4742efc6fe68d528f3ab13fa6e9dcc Bug 1337189 (part 25) - Move streaming/saving code out of Sampler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/8c1f662d5bc132611069fe0bbfa5c77ea8be1ed4 Bug 1337189 (part 26) - Remove Sampler.cpp. r=mstange.
Assignee | ||
Updated•7 years ago
|
Keywords: leave-open
Comment 49•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fc0ad9814a6c https://hg.mozilla.org/mozilla-central/rev/8694332bf02a https://hg.mozilla.org/mozilla-central/rev/9d9c49975539 https://hg.mozilla.org/mozilla-central/rev/4a894b2316f0 https://hg.mozilla.org/mozilla-central/rev/99c10f2d0d76 https://hg.mozilla.org/mozilla-central/rev/6a94811a6ef0 https://hg.mozilla.org/mozilla-central/rev/778675533fb9 https://hg.mozilla.org/mozilla-central/rev/d4c677b0b92f https://hg.mozilla.org/mozilla-central/rev/63172ee12b03 https://hg.mozilla.org/mozilla-central/rev/6c209f8f314b https://hg.mozilla.org/mozilla-central/rev/3b603e66e276 https://hg.mozilla.org/mozilla-central/rev/5805116bb2e0 https://hg.mozilla.org/mozilla-central/rev/aa0f4dd3ce47 https://hg.mozilla.org/mozilla-central/rev/8c1f662d5bc1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Assignee | ||
Updated•7 years ago
|
Blocks: profiler-cleanup
Comment 50•7 years ago
|
||
Comment on attachment 8835339 [details] [diff] [review] (part 16) - Move all the bools out of Sampler This patch causes build error while enabling TaskTracer. >15:00.54 In file included from mozilla-central/obj-firefox.noindex/tools/profiler/Unified_cpp_tools_profiler0.cpp:74: >15:00.54 mozilla-central/tools/profiler/core/platform.cpp:2053:7: error: use of undeclared identifier 'mTaskTracer'; did you mean 'gTaskTracer'? >15:00.54 if (mTaskTracer) { >15:00.54 ^~~~~~~~~~~ >15:00.54 gTaskTracer >15:00.54 mozilla-central/tools/profiler/core/platform.cpp:163:13: note: 'gTaskTracer' declared here >15:00.54 static bool gTaskTracer = false; >15:00.54 ^ >15:00.54 mozilla-central/tools/profiler/core/platform.cpp:2214:7: error: use of undeclared identifier 'mTaskTracer'; did you mean 'gTaskTracer'? >15:00.54 if (mTaskTracer) { >15:00.54 ^~~~~~~~~~~ >15:00.54 gTaskTracer >15:00.54 mozilla-central/tools/profiler/core/platform.cpp:163:13: note: 'gTaskTracer' declared here >15:00.54 static bool gTaskTracer = false; >15:00.54 ^ >15:00.54 2 errors generated.
Flags: needinfo?(n.nethercote)
Assignee | ||
Comment 51•7 years ago
|
||
Attachment #8836582 -
Flags: review?(schien)
Comment 52•7 years ago
|
||
Comment on attachment 8836582 [details] [diff] [review] (follow-up) - Fix MOZ_TASK_TRACER bustage Review of attachment 8836582 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8836582 -
Flags: review?(schien) → review+
Assignee | ||
Comment 53•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/96caedfc0c821b181ba811f3a550ca9ed7e06434 Bug 1337189 (follow-up) - Fix MOZ_TASK_TRACER bustage. r=schien.
Comment 54•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/96caedfc0c82
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(n.nethercote)
You need to log in
before you can comment on or make changes to this bug.
Description
•