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.

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.

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.

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?

(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 :).