Benoit Girard (:BenWa) Assignee
	Description • 11 years ago

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.

	Benoit Girard (:BenWa) Assignee
	Comment 1 • 11 years ago

Comment on attachment 807767 [details] [diff] [review]
patch

https://tbpl.mozilla.org/?tree=Try&rev=fccd929a4b47

	(no longer active)
	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

	Benoit Girard (:BenWa) Assignee
	Comment 3 • 11 years ago

https://tbpl.mozilla.org/?tree=Try&rev=b96fce78ec36

	Benoit Girard (:BenWa) Assignee
	Comment 4 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/cd28a8561332

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 5 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/cd28a8561332

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧
	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.

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 11 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/6d32335e66e7