1323605 - Consider removing MOZ_ENABLE_PROFILER_SPS (making it the default)

Status: RESOLVED WONTFIX

(Core :: Gecko Profiler, defect)

Description

[Markus Stange [:mstange]]

MOZ_ENABLE_PROFILER_SPS is defined under these circumstances:

@depends(target)
def sps_profiler(target):
    if target.os == 'Android':
        return target.cpu in ('arm', 'x86')
    elif target.kernel == 'Linux':
        return target.cpu in ('x86', 'x86_64')
    return target.os in ('OSX', 'WINNT')

I think this exactly matches all of our Tier 1 platforms.

Getting rid of this this define would allow us to simplify a lot of the indirection in GeckoProfiler.h and friends.

Comment 1

[Nathan Froyd [:froydnj]]

This would essentially break all of our non-Tier 1 platforms without any recourse, right?  They have to be able to define MOZ_DISABLE_PROFILER_SPS or equivalent, or they have to write they own stub, do-nothing functions.  Or do I misunderstand your proposal?

Comment 2

[Markus Stange [:mstange]]

Oops, no I had just forgotten for a moment how much platform-specific code there is in the profiler. I was under the assumption that we already have a fallback implementation and that most of the differences between the platforms is in the actual stackwalking. But you're right - the change as proposed would break all non-Tier 1 platforms.

So it seems we need to make the profiler share more code across platforms and use existing MFBT / xpcom / nspr abstractions for the things which are currently platform-specific, and then make sure that there is a working base implementation that works on all platforms.

Comment 3

[Nicholas Nethercote [inactive]]

(In reply to Markus Stange [:mstange] (away until Feb 22) from comment #0)
> 
> Getting rid of this this define would allow us to simplify a lot of the
> indirection in GeckoProfiler.h and friends.

Bug 1332577 simplified that indirection greatly. mstange, do you think it's still worth having this bug open?

Comment 4

[Markus Stange [:mstange]]

Probably not.