OS/arch/platform detection is a mess in the profiler. There is absolutely no consistency about which macros are used. I want to clean this up.

This change increases consistency: - Each OS is dealt with one at a time (no more interleaving). - For each OS, x86 is now the first listed architecture. The patch also adds the missing "#undef SPS_PLAT_x86_android".

This factors out some common preprocessor conditions.

They duplicate the equivalent SPS_* macros. (The SPS_* macros have already crept into use in some places within LUL.)

SPS_STANDALONE was removed in bug 1317771.

This factors out some common preprocessor conditions.

Nice tidyup! Yes it was a mess. One comment: do you still want to use the "SPS_" prefix, or should we change that to something else? For some reason I have the impression that it stands for "Simple Profiling System", but now I thought we decided to use the name "Gecko Profiler" for the profiler as a whole.

Yes, these macros are the only remaining use of SPS left in the code. It would be good to remove them, but it's nice that SPS is such a short prefix. Maybe PROF_* would be good? Or GP_* for "Gecko Profiler"?

I would vote for GP_, on the basis that PROF_ is a bit too generic.

Comment on attachment 8837421 [details] [diff] [review] (part 4) - Remove a stray, misspelled SPS_STANDALNOE use Review of attachment 8837421 [details] [diff] [review]: ----------------------------------------------------------------- lol

https://hg.mozilla.org/integration/mozilla-inbound/rev/9d9a3692b012221e74ee6fbc70af95dff83e1a7d Bug 1339695 (part 1) - Remove LUL_{ARCH,OS,PLAT}_* macros. r=jseward. https://hg.mozilla.org/integration/mozilla-inbound/rev/f6869c7b0707091d551285243c43993d7b235265 Bug 1339695 (part 2) - Reorder PlatformMacros.h.h. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/b735a190b14f0a5ea0f0ccb0793624dd89291055 Bug 1339695 (part 3) - Introduce USE_FAULTY_LIB. r=jseward. https://hg.mozilla.org/integration/mozilla-inbound/rev/002e9c45f121f951f205cc3d3ec7fb563f5a831c Bug 1339695 (part 4) - Remove a stray, misspelled SPS_STANDALNOE use. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/6205142186156da59305508759cd08982e38ae1b Bug 1339695 (part 5) - Introduce PROFILE_JAVA. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/eb3b5ef730656f41a31d5b537a83553337c4a549 Bug 1339695 (part 6) - Remove some B2G-only code in profiler_register_thread(). r=mstange.

It's defined if any of XP_{WIN,MAC,LINUX} are defined and the latter includes Android as well. So it's defined on all the OSes the profiler supports.

Currently we use the SPS_* macros in some places, but also use other ones like __arm__ and ANDROID and XP_{WIN,MAC,LINUX}. This patch makes the profiler consistently use the SPS_* macros and removes the V8_HOST_ARCH_* macros. The patch also does the following. - Cleans up some header inclusions, e.g. including pthread.h directly in the files that use it, and removing some unneeded android/log.h inclusions. - Removes an unused branch in SetSampleContext() -- we don't support ARM on anything other than Android, and glibc 2.3 is ancient. - Doesn't use SPS_* in PseudoStack.h because that would require exporting PlatformMacros.h, which doesn't seem worthwhile. Some things that aid the understanding of this patch. - XP_LINUX and LINUX are both defined for Linux *and* Android. - x86/Android is the only supported platform that doesn't define HAVE_NATIVE_UNWIND. - Every platform that defines USE_LUL_STACKWALK also defines HAVE_NATIVE_UNWIND.

Specifically: - platform-linux.cc -> platform-linux-android.cpp - platform-macos.cc -> platform-macos.cpp - platform-win32.cc -> platform-win32.cpp Adding "android" to the first one is the most important part, because it makes things clearer. The .cc to .cpp change is less important but I might as well do it while I'm in here.

This removes the final mentions of the old "SPS" name.

Comment on attachment 8837907 [details] [diff] [review] (part 9) - Rename the platform-* files I was wondering when the cc -> cpp change would happen :)

We don't need OS now that the platform-*.cpp files are in the same compilation unit as platform.cpp. The patch removes the sleep functions because they are unnecessary indirection. OS::Startup() is necessary, but the patch renames it PlatformInit() to match Platform{Start,Stop}() and profiler_init(), from which it is called.

https://hg.mozilla.org/mozilla-central/rev/9d9a3692b012 https://hg.mozilla.org/mozilla-central/rev/f6869c7b0707 https://hg.mozilla.org/mozilla-central/rev/b735a190b14f https://hg.mozilla.org/mozilla-central/rev/002e9c45f121 https://hg.mozilla.org/mozilla-central/rev/620514218615 https://hg.mozilla.org/mozilla-central/rev/eb3b5ef73065

mstange: not sure if you overlooked part 11 or if you're just biding your time :)

Oops, looks like I closed the tab that I had opened for part 11 accidentally and then lost track of it.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8cc7d383df21ee79c29dc8ab0942f676569cd1c4 Bug 1339695 (part 7) - Remove ENABLE_LEAF_DATA. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/33c5f636b414db151ddbb6a865878fe53b43e11b Bug 1339695 (part 8) - Clean up platform detection throughout the profiler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/ec0490c4ada57ffc53da86806021177585ab5343 Bug 1339695 (part 9) - Rename the platform-* files. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/9eed66e9369a5151a26471a0786aad6a41d96fc8 Bug 1339695 (part 10) - Rename SPS_* macros as GP_*. r=jseward. https://hg.mozilla.org/integration/mozilla-inbound/rev/12fae55fe512dfd640b09279821d49b5f9e03a8a Bug 1339695 (part 11) - Remove the profiler's OS class. r=mstange.

https://hg.mozilla.org/mozilla-central/rev/8cc7d383df21 https://hg.mozilla.org/mozilla-central/rev/33c5f636b414 https://hg.mozilla.org/mozilla-central/rev/ec0490c4ada5 https://hg.mozilla.org/mozilla-central/rev/9eed66e9369a https://hg.mozilla.org/mozilla-central/rev/12fae55fe512