This will get called more with bug 721543, so we should get coverage of it in the SPS.
Created attachment 592267 [details] [diff] [review] add SPS labels for all major CC entry points This covers all the major entry points, eg those that take any time at all. In my testing, forgetSkippable takes less than a ms, but it isn't only called every 250ms, so I don't imagine the extra overhead of a label will be too bad. I want to add it to the profiler in case it could become a large source of time under some circumstances. I haven't tested this yet, but I can't imagine any problems.
Sounds ok to me. forgetSkippable isn't particularly hot code. But, is it ok to #include "sampler.h" in xpcom? Should it at least be #ifdef'ed?
(In reply to Olli Pettay [:smaug] from comment #2) > But, is it ok to #include "sampler.h" in xpcom? > Should it at least be #ifdef'ed? Why wouldn't it be okay? It is already used in one place in xpcom, in xpcom/threads/nsTimerImpl.cpp I'm not sure what you mean about being #ifdef'ed?
Because xpcom can be used in code which doesn't use tools/ I have no idea why it would be ok to #include sampler.h in xpcom by default. Benjamin might know.
Ah, that's interesting. In the worst case I can lift this out to nsJSEnvironment or whatnot, but there are a few callsites, and we'd have to remember to add the label to any new ones, which is why I was doing it like this.
Apart from the glue, which is special, this can rely on the profiler, that's not a problem. However, I really think that the header name "sampler.h" is very generic... can we make it mozilla/sampler.h as with most of our other new headers?
That's a good idea. I'll get a patch ready to apply this to the whole project.
Comment on attachment 592267 [details] [diff] [review] add SPS labels for all major CC entry points Sounds like maybe this is okay then? Just r- if I'm misunderstanding.