Closed Bug 1512806 Opened 6 years ago Closed 5 years ago

Pass the correct profile generate flags when building the instrumented build on android

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: chmanchester, Assigned: mshal)

References

Details

Attachments

(1 file, 2 obsolete files)

Bug 1512806 - Pass correct profile flags for Android PGO; r?#firefox-build-system-reviewers
47 bytes, text/x-phabricator-request
Details | Review 
According to the documentation I've found Android supports `-fprofile-generate` on Android but not `-fprofile-instr-generate`. The current logic for passing flags assumes it's not going to do an Android build and passes `-fprofile-instr-generate`.
Attachment #9030087 - Attachment is obsolete: true

According to the documentation I've found

What documentation? That might only apply to the compiler shipped in the Android NDK, and it might even date from when that compiler was GCC.

(In reply to Mike Hommey [:glandium] from comment #2)

According to the documentation I've found

What documentation? That might only apply to the compiler shipped in the
Android NDK, and it might even date from when that compiler was GCC.

https://source.android.com/devices/tech/perf/pgo

In particular, "While Clang supports both AST-based (-fprofile-instr-generate) and LLVM IR-based (-fprofile-generate), Android supports only LLVM IR-based for instrumentation-based PGO."

On its own, that comment makes no sense. Especially when later on, they still talk about profile-instr-use. Did you try not changing the pgo flags?

(In reply to Mike Hommey [:glandium] from comment #4)

On its own, that comment makes no sense. Especially when later on, they still talk about profile-instr-use. Did you try not changing the pgo flags?

I gave this a try today. On the instrumented build I changed -fprofile-generate=/data/local/tmp to -fprofile-instr-generate=/data/local/tmp, but when I trigger the hook to call __llvm_profile_dump(), no profraw file is generated.

Is __llvm_profile_dump() not the right hook to call in this case? We have to call it manually since the atexit() handlers don't get called on Android.

Assignee: cmanchester → mshal

Android requires us to use -fprofile-generate / -fprofile-use instead of
-fprofile-instr-generate / -fprofile-instr-use. Additionally, we have
to pass in the path to write out the profile data as /data/local/tmp,
since the cwd from where the Fennec process is invoked is not writeable.

Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b8c7842e8b1a
Pass correct profile flags for Android PGO; r=firefox-build-system-reviewers,chmanchester
Backout by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/903a04ef7adc
Backed out changeset b8c7842e8b1a for Windows 2012 bustages complaning about gen_cflags CLOSED TREE

Backed out changeset b8c7842e8b1a (Bug 1512806) for Windows 2012 bustages complaning about gen_cflags

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=windows%2C2012%2Cdebug%2Cbuild-win32-rusttests%2Fdebug%2C%28br%29&tochange=903a04ef7adcece77a10381b169c40b0831aa529&fromchange=106c2c83d88328c630ecf0fd0b8f95f9e47b5d2a

Backout link: https://hg.mozilla.org/integration/autoland/rev/903a04ef7adcece77a10381b169c40b0831aa529

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=229803352&repo=autoland&lineNumber=1210

02:55:56 INFO - checking for the host C++ compiler... z:/build/build/src/clang/bin/clang-cl.exe
02:55:56 INFO - checking whether the host C++ compiler can be used... yes
02:55:56 INFO - checking the host C++ compiler version... 8.0.0
02:55:56 INFO - checking the host C++ compiler works... yes
02:55:56 INFO - checking for 64-bit OS... no
02:55:57 ERROR - Traceback (most recent call last):
02:55:57 INFO - File "z:/build/build/src/configure.py", line 132, in <module>
02:55:57 INFO - sys.exit(main(sys.argv))
02:55:57 INFO - File "z:/build/build/src/configure.py", line 38, in main
02:55:57 INFO - sandbox.run(os.path.join(os.path.dirname(file), 'moz.configure'))
02:55:57 INFO - File "z:\build\build\src\python\mozbuild\mozbuild\configure_init_.py", line 465, in run
02:55:57 INFO - func(*args)
02:55:57 INFO - File "z:\build\build\src\python\mozbuild\mozbuild\configure_init_.py", line 509, in _value_for
02:55:57 INFO - return self.value_for_depends(obj)
02:55:57 INFO - File "z:\build\build\src\python\mozbuild\mozbuild\util.py", line 947, in method_call
02:55:57 INFO - cache[args] = self.func(instance, *args)
02:55:57 INFO - File "z:\build\build\src\python\mozbuild\mozbuild\configure_init.py", line 518, in value_for_depends
02:55:57 INFO - value = obj.result()
02:55:57 INFO - File "z:\build\build\src\python\mozbuild\mozbuild\util.py", line 947, in method_call
02:55:57 INFO - cache[args] = self.func(instance, *args)
02:55:57 INFO - File "z:\build\build\src\python\mozbuild\mozbuild\configure_init.py", line 153, in result
02:55:57 INFO - return self.func(*resolved_args)
02:55:57 INFO - File "z:\build\build\src\python\mozbuild\mozbuild\configure_init.py", line 1086, in wrapped
02:55:57 INFO - return new_func(*args, **kwargs)
02:55:57 INFO - File "z:/build/build/src/build/moz.configure/toolchain.configure", line 1452, in pgo_flags
02:55:57 INFO - gen_cflags=gen_cflags,
02:55:57 INFO - UnboundLocalError: local variable 'gen_cflags' referenced before assignment
02:55:57 INFO - *** Fix above errors and then restart with
02:55:57 INFO - "./mach build"
02:55:57 INFO - client.mk:111: recipe for target 'configure' failed
02:55:57 INFO - mozmake.EXE: *** [configure] Error 1
02:55:57 ERROR - Return code: 2
02:55:57 WARNING - setting return code to 2
02:55:57 FATAL - 'mach build -v pre-export export recurse_rusttests' did not run successfully. Please check log for errors.
02:55:57 FATAL - Running post_fatal callback...
02:55:57 FATAL - Exiting -1
02:55:57 INFO - [mozharness: 2019-02-22 02:55:57.139000Z] Finished build step (failed)

Flags: needinfo?(mshal)

Android requires us to use -fprofile-generate / -fprofile-use instead of
-fprofile-instr-generate / -fprofile-instr-use. Additionally, we have
to pass in the path to write out the profile data as /data/local/tmp,
since the cwd from where the Fennec process is invoked is not writeable.

Per phabricator, we're going to use LLVM_PROFILE_FILE in the environment instead of changing the flags here.

Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(mshal)
Resolution: --- → WONTFIX
Attachment #9047239 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: