Closed Bug 1576263 Opened 5 months ago Closed 5 months ago

modules/libpref/Preferences.cpp:17:10: fatal error: 'ProfilerMarkerPayload.h' file not found

Categories

(Core :: Preferences: Backend, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox70 --- fixed
firefox71 --- fixed

People

(Reporter: jbeich, Assigned: mstange)

References

(Regression)

Details

Attachments

(1 file, 1 obsolete file)

GeckoProfiler is not supported on Tier3 platforms: Linux powerpc/sparc/s390x, DragonFly/FreeBSD/NetBSD/OpenBSD, Solaris. Until bug 1371159 reproducing on Tier1 requires hacking gecko_profiler() in toolkit/moz.configure to return None.

$ c++ --version
FreeBSD clang version 8.0.1 (tags/RELEASE_801/final 366581) (based on LLVM 8.0.1)
Target: x86_64-unknown-freebsd13.0
Thread model: posix
InstalledDir: /usr/bin
$ ./mach bootstrap
$ ./mach build
[...]
In file included from objdir/modules/libpref/Unified_cpp_modules_libpref0.cpp:2:
modules/libpref/Preferences.cpp:17:10: fatal error: 'ProfilerMarkerPayload.h' file not found
#include "ProfilerMarkerPayload.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Is there a compile-time constant/macro that you recommend using to make sure that we don't include this functionality on these platforms? Sorry about the breakage.

Flags: needinfo?(jbeich)

Looks like I can use MOZ_GECKO_PROFILER. Does that jive with your understanding, Jan?

(In reply to Will Hawkins from comment #2)

Looks like I can use MOZ_GECKO_PROFILER. Does that jive with your understanding, Jan?

Yes. Also, after bug 1403868 some macro wrappers can be used unconditionally.

Flags: needinfo?(jbeich)

Not enough to fix --enable-warnings-as-errors yet. Sorry, I can't log in (to review) on Phabricator due to bug 1536716.

In file included from objdir/modules/libpref/Unified_cpp_modules_libpref0.cpp:2:
modules/libpref/Preferences.cpp:4330:13: error: unused function 'PrefValueToString' [-Werror,-Wunused-function]
static void PrefValueToString(const bool* b, nsCString& value) {
            ^
modules/libpref/Preferences.cpp:4333:13: error: unused function 'PrefValueToString' [-Werror,-Wunused-function]
static void PrefValueToString(const int* i, nsCString& value) {
            ^
modules/libpref/Preferences.cpp:4336:13: error: unused function 'PrefValueToString' [-Werror,-Wunused-function]
static void PrefValueToString(const uint32_t* u, nsCString& value) {
            ^
modules/libpref/Preferences.cpp:4339:13: error: unused function 'PrefValueToString' [-Werror,-Wunused-function]
static void PrefValueToString(const float* f, nsCString& value) {
            ^
modules/libpref/Preferences.cpp:4342:13: error: unused function 'PrefValueToString' [-Werror,-Wunused-function]
static void PrefValueToString(const nsACString& s, nsCString& value) {
            ^
5 errors generated.

Let me know if that updated patch is better :-) Sorry about earlier!

Flags: needinfo?(jbeich)

Comment on attachment 9087873 [details]
Bug 1576263: Conditionally include profiler functionality only where it's supported.

Builds fine on FreeBSD.

Flags: needinfo?(jbeich)
Attachment #9087873 - Flags: feedback+

Thank you for confirming, Jan!

This also fixes it on Linux on ppc64le.

Can this land? We're broken on beta without it.

I'll press the button.

Oh, never mind, it's not been r+ed yet. I thought njn had already reviewed it in my absence.

Yeah, it would be great to have Nick sign off.

Will, I've attached an alternative patch that only has one #ifdef per lookup function instead of three. Please take a look and let me know what you think of it. This patch is only adding markers when pref_Lookup/pref_SharedLookup succeeds, so it assumes that lookup failures only happen in unexpected error cases and are not relevant for general performance profiling.

Attachment #9087873 - Attachment is obsolete: true
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/c01f03f68def
Conditionally include profiler functionality only where it's supported. r=whawkins,njn
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Assignee: nobody → mstange

Can you backport to Beta 70 as well, chasing bug 1551313?

Flags: needinfo?(mstange)

I expect this to apply cleanly on 70. I'll request uplift approval.

Flags: needinfo?(mstange)

Comment on attachment 9089787 [details]
Bug 1576263 - Conditionally include profiler functionality only where it's supported. r=whawkins,njn

Beta/Release Uplift Approval Request

  • User impact if declined: Tier3 platforms have broken builds. No user impact for Firefox on Tier1 platforms.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Extremely low risk. It's pretty much only a build fix. There is one small behavior change but it only affects whether a profiler marker is emitted in a certain error case.
  • String changes made/needed:
Attachment #9089787 - Flags: approval-mozilla-beta?

Comment on attachment 9089787 [details]
Bug 1576263 - Conditionally include profiler functionality only where it's supported. r=whawkins,njn

Mostly a build fix; we're in very early beta so I'm fine with this uplift to support Tier 3

Attachment #9089787 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.