On B2G the profiler was started from a signal handler. I don't think we need this ability any more.

Markus, I'm unclear about this. On x86_64-linux ProfilerSignalHandler is definitely used -- I put a printf in to check. Did you mean ProfilerSaveSignalHandler?

I can't tell for sure whether ProfilerSaveSignalHandler is used or not, because it's only reachable by delivery of SIGUSR2, and various other parts of the system also use SIGUSR2. Superficially at least though it looks like those other uses are unrelated. Assuming that's the case, there's a whole bunch of stuff that can be removed: remove ProfilerSaveSignalHandler() -> RequestSave() is never called -> RequestSave() can be removed -> GeckoSampler::mSaveRequested can never become true -> GeckoSampler::HandleSaveRequest is always a no-op -> GeckoSampler::HandleSaveRequest can be removed -> no instance of class SaveProfileTask is ever made -> all of SaveProfileTask and things only reachable from there, can be removed

Removes everything mentioned above, up to but not including SaveProfileTask.

Yes, sorry, I meant ProfilerSaveSignalHandler. I'm pretty sure there aren't any other users of the profiler save signal. It was used by B2G's profile.sh here: https://github.com/mozilla-b2g/B2G/blob/master/profile.sh#L557 I'm curious, why did you choose not to remove SaveProfileTask? I think it should be removed as well.

(In reply to Markus Stange [:mstange] from comment #4) > I'm curious, why did you choose not to remove SaveProfileTask? I think it > should be removed as well. Only because I wanted to check I was on the right track before spending more time on this. Now that you've confirmed that, I will definitely remove SaveProfileTask as well.

Removes: ProfilerSaveSignalHandler() RequestSave() GeckoSampler::mSaveRequested GeckoSampler::HandleSaveRequest() class SaveProfileTask, all of it This leaves class ProfileSaveEvent stranded in SaveProfileTask.{h,cpp}. It is not much code and only used in Sampler.cpp. So I moved it to to Sampler.cpp and removed the files SaveProfileTask.{h,cpp} entirely.

Comment on attachment 8832367 [details] [diff] [review] bug1328916-3.cset Review of attachment 8832367 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/profiler/core/platform-linux.cc @@ -434,5 @@ > - if (sigaction(SIGNAL_SAVE_PROFILE, &sa2, &old_sigsave_signal_handler_) != 0) { > - LOG("Error installing start signal"); > - return; > - } > - LOG("Signal installed"); You might want to leave that last LOG statement in, because it arguably applies to the ProfilerSignalHandler setup above.

Pushed by jseward@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/296fcb87d350 Remove ProfilerSaveSignalHandler and transitively reachable code. r=mstange.

(In reply to Nicholas Nethercote [:njn] from comment #7) > You might want to leave that last LOG statement in, [..] Ah yes, good point. Fixed. It was in fact there in an earlier version of the patch but appears to have been lost during rebasing.

(In reply to Julian Seward [:jseward] from comment #1) > Markus, I'm unclear about this. On x86_64-linux ProfilerSignalHandler is > definitely used -- I put a printf in to check. > > Did you mean ProfilerSaveSignalHandler? It turns out that I meant both SigstartHandler and ProfilerSaveSignalHandler. This bug took care of the latter, so I've filed bug 1351946 for the former.