Closed Bug 914815 Opened 6 years ago Closed 6 years ago

Support profiler options when starting the profiler with a signal

Categories

(Core :: Gecko Profiler, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: BenWa, Assigned: BenWa)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
No description provided.
Attachment #802547 - Flags: review?(jld)
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 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.
Attachment #802547 - Flags: review?(jld) → review+
Blocks: 914654
Attached patch patch v2Splinter Review
Assignee: nobody → bgirard
Attachment #802547 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #803207 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/b4c41406c202
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.