914815 - Support profiler options when starting the profiler with a signal

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Benoit Girard (:BenWa)]

Attachment: patch

Comment 1

[Benoit Girard (:BenWa)]

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

Review of attachment 802547 [details] [diff] [review]:
-----------------------------------------------------------------

::: tools/profiler/platform-linux.cc
@@ +351,5 @@
>    if (pthread_create(
>          &signal_sender_thread_, NULL, SignalSender, NULL) == 0) {
>      signal_sender_launched_ = true;
>    }
> +  pthread_setname_np(signal_sender_thread_, "SamplerThread");

Opps ignore this change

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

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

Review of attachment 802547 [details] [diff] [review]:
-----------------------------------------------------------------

::: tools/profiler/platform-linux.cc
@@ +426,3 @@
>  
>  static void StartSignalHandler(int signal, siginfo_t* info, void* context) {
> +

We've discussed the async signal unsafety that's going on here, but could there be a comment warning about it?

@@ +448,5 @@
> +    printf_stderr("Profiling only %s\n", threadName);
> +  }
> +  infile.close();
> +
> +  profiler_start(PROFILE_DEFAULT_ENTRY, 1.0,

The second argument's type seems to be double if the profiler is disabled (GeckoProfiler.h) and int if it is (GeckoProfilerImpl.h), so passing 1.0 is a little confusing.

Comment 3

[Benoit Girard (:BenWa)]

Attachment: patch v2

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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