Closed
Bug 918833
Opened 11 years ago
Closed 11 years ago
Improved profiler thread register
Categories
(Core :: Gecko Profiler, defect)
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: BenWa, Assigned: BenWa)
Details
(Whiteboard: [qa-])
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
BenWa
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Currently you have to uncomment the thread register on b2g. I quickly disabled it at the time because registering a thread allocates a pseudostack which requires memory and we weren't using this feature at all. Now I want to find a solution to use this feature without a custom build. Let's enable it for profiling builds or enable it for builds that run with MOZ_PROFILER_STARTUP. Also this fixes the bug where registering the same thread would mess things up.
Attachment #807767 -
Flags: review?(ehsan)
Assignee | ||
Updated•11 years ago
|
Attachment #807767 -
Attachment is patch: true
Attachment #807767 -
Attachment mime type: text/x-patch → text/plain
Assignee | ||
Comment 1•11 years ago
|
||
Comment on attachment 807767 [details] [diff] [review] patch https://tbpl.mozilla.org/?tree=Try&rev=fccd929a4b47
Comment 2•11 years ago
|
||
Comment on attachment 807767 [details] [diff] [review] patch Review of attachment 807767 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/profiler/platform-linux.cc @@ +401,5 @@ > + if (info->ThreadId() == id) { > + // Thread already registered. This means the first unregister will be > + // too early. > + ASSERT(false); > + return; return false ::: tools/profiler/platform-macos.cc @@ +364,5 @@ > + if (info->ThreadId() == id) { > + // Thread already registered. This means the first unregister will be > + // too early. > + ASSERT(false); > + return; return false ::: tools/profiler/platform-win32.cc @@ +284,5 @@ > + if (info->ThreadId() == id) { > + // Thread already registered. This means the first unregister will be > + // too early. > + ASSERT(false); > + return; return false
Attachment #807767 -
Flags: review?(ehsan) → review+
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → bgirard
Assignee | ||
Comment 3•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=b96fce78ec36
Attachment #807767 -
Attachment is obsolete: true
Attachment #809367 -
Flags: review+
Assignee | ||
Comment 4•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd28a8561332
Attachment #809367 -
Attachment is obsolete: true
Attachment #810039 -
Flags: review+
https://hg.mozilla.org/mozilla-central/rev/cd28a8561332
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Updated•11 years ago
|
status-b2g-v1.2:
--- → affected
Comment 6•11 years ago
|
||
I've crossported the patch to Gecko 26, which is also affected. (It doesn't apply cleanly as-is.) We'll probably want the profiler working as effortlessly as possible as B2G 1.2 nears release. [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Developers will have to apply this patch (or equivalent) manually if they want to profile non-main threads on aurora. Testing completed (on m-c, etc.): Successfully profiled the B2G Compositor thread, with this patch on aurora. Risk to taking this patch (and alternatives if risky): None if profiling isn't enabled, as I understand it. String or IDL/UUID changes made by this patch: None.
Attachment #813876 -
Flags: review?(bgirard)
Attachment #813876 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•11 years ago
|
Attachment #813876 -
Flags: review?(bgirard) → review+
Updated•11 years ago
|
Attachment #813876 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 7•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/6d32335e66e7
Updated•11 years ago
|
Whiteboard: [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•