Closed Bug 707185 Opened 8 years ago Closed 8 years ago

Programmatic control for eventtracer

Categories

(Core :: Gecko Profiler, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: BenWa, Assigned: BenWa)

References

Details

Attachments

(1 file, 2 obsolete files)

We need programmatic control to turn on/off event tracing at run time. This will let users use it for profiling without having to change their environment.
Attached patch patch (obsolete) — Splinter Review
I have this patch here. It has a couple problems:
(1) If InitEventTracing is called too early on startup it never begins tracing events.
(2) It should keep a start/stop count so that it can have multiple users.
Depends on: 683229
No longer depends on: 700754
Attached patch patch (obsolete) — Splinter Review
This patch fixes the first problem. Stopped the tracing thread while gecko is running results in sigsegv in WidgetTraceEvent.mm:77 (Mac). For now well init the tracing tread when the profiler is started and leave it running.
Attachment #578602 - Attachment is obsolete: true
Attached patch patchSplinter Review
Attachment #578964 - Attachment is obsolete: true
Attachment #578965 - Flags: review?(ted.mielczarek)
Comment on attachment 578965 [details] [diff] [review]
patch

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

I'm a little worried about the potential for crashes/hangs by starting/stopping multiple times, since it's kind of fiddly, but I guess we can cross that bridge if we hit it. We definitely had a shutdown crash on Linux on tinderbox, we disabled responsiveness instrumentation on Linux for now, FYI.

::: toolkit/xre/nsAppRunner.cpp
@@ +3546,5 @@
>            nativeApp->Enable();
>          }
>  
>  #ifdef MOZ_INSTRUMENT_EVENT_LOOP
> +        if (PR_GetEnv("MOZ_INSTRUMENT_EVENT_LOOP") || SAMPLER_IS_ACTIVE()) {

I take it there's some way to start with SPS enabled?
Attachment #578965 - Flags: review?(ted.mielczarek) → review+
Assignee: nobody → bgirard
(In reply to Ted Mielczarek [:ted, :luser] from comment #4)
> Comment on attachment 578965 [details] [diff] [review]
> patch
> 
> Review of attachment 578965 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm a little worried about the potential for crashes/hangs by
> starting/stopping multiple times, since it's kind of fiddly, but I guess we
> can cross that bridge if we hit it. We definitely had a shutdown crash on
> Linux on tinderbox, we disabled responsiveness instrumentation on Linux for
> now, FYI.

I'm worried about that as well but since people will only run into this if they opt into profiling for now I think it's acceptable. I think we'll probably have the profiler add-on turn it on when you start your first profiling and leave it running.

Fixing this for Linux would be useful. Will someone be looking into that? It's not needed right now since I think we will focus on Windows, Mac and native fennec.

> 
> ::: toolkit/xre/nsAppRunner.cpp
> @@ +3546,5 @@
> >            nativeApp->Enable();
> >          }
> >  
> >  #ifdef MOZ_INSTRUMENT_EVENT_LOOP
> > +        if (PR_GetEnv("MOZ_INSTRUMENT_EVENT_LOOP") || SAMPLER_IS_ACTIVE()) {
> 
> I take it there's some way to start with SPS enabled?

Yes, I tried setting up the event tracing when the profiler starts but this happens before the event loop so I needed to hook in here.

The recorded profile shows that the app isn't responsive until then, which is true :).
https://hg.mozilla.org/mozilla-central/rev/dd24fab5fc63
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Component: General → Gecko Profiler
Product: Toolkit → Core
You need to log in before you can comment on or make changes to this bug.