1634785 - De-duplicate SharedLibraries code - use BaseProfiler implementation in Gecko Profiler code

Status: RESOLVED FIXED

(Core :: Gecko Profiler, task, P1)

Description

[Markus Stange [:mstange]]

We currently have two implementations of SharedLibraries: https://searchfox.org/mozilla-central/search?q=&path=shared-libraries

Once Gecko profiler code can depend on BaseProfiler code, we can unify them.

Comment 1

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1634785 - Move SharedLibrary IPC ParamTraits outside of shared library header file r?aabh,mstange

We will remove the libxul implementation soon and use the one in mozglue.
We can't keep these structs inside the mozglue, so we have to move them to
somewhere else in libxul. This file made the most sense since it's the one who
implements their methods.

Comment 2

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1634785 - Add missing shared libraries methods to the baseprofiler implementation r?aabh,mstange

These methods were already inside the shared libraries that was in
tools/profiler. Adding these methods here to make the implementations closer
and make the deduplication easier in the following commits.

Comment 3

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1634785 - Make the base profiler shared libraries file closer to gecko profiler one r?aabh,mstange

Again removing some unused/unneeded things so it's going to be easier to
deduplicate the files.

Comment 4

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1634785 - Fix how shared library names are found on linux and macOS r?aabh,mstange

While testing the de-duplication on Linux and macOS, I noticed that the
symbolication was completely broken due to having incorrect library names in
libs array. They were always like: "<obj-dir>/dist/bin/libxul.so" instead of
"libxul.so". The library names are found from their paths, and apparently we
were using an incorrect path separator for Linux and macOS, "" instead of "/".

Comment 5

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1634785 - Do not expose AddSharedLibraryFromModuleInfo that's only used internally r?aabh,mstange

This method was used only internally, and having this method declaration causes
issues. That's this method was already like this in tools/profiler version of
this but it wasn't implemented here.

Comment 6

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1634785 - Deduplicate shared libraries code r?aabh,mstange

This patch finally removes the shared libraries code that was inside
tools/profiler and uses the one in base profiler everywhere.

Comment 7

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1634785 - Rename BaseProfilerSharedLibraries.h to SharedLibraries.h r?aabh,mstange

Now that we only have one implementation of this code, we don't have to
differentiate it with "BaseProfiler".

Comment 8

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1634785 - Return early for ntdll.dll if EAF+ is enabled on mozglue SharedLibrary as well r?gstoll!,aabh,mstange

We already had this early return on the shared-libraries code that was on the
tools/profiler directory. Adding this check here as well to keep the both files
in sync. It will help us deduplicating the shared libraries code later.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf73d2f240f1
Move SharedLibrary IPC ParamTraits outside of shared library header file r=mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/2abc88b08f69
Add missing shared libraries methods to the baseprofiler implementation r=mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/f9b6ae065ffc
Make the base profiler shared libraries file closer to gecko profiler one r=mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/32b940c88cca
Fix how shared library names are found on linux and macOS r=mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/42d0181c117a
Do not expose AddSharedLibraryFromModuleInfo that's only used internally r=mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/1c43270bdcaf
Return early for ntdll.dll if EAF+ is enabled on mozglue SharedLibrary as well r=mstange,profiler-reviewers,bobowen
https://hg.mozilla.org/integration/autoland/rev/cb492d775d37
Deduplicate shared libraries code r=mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/3ea11e90d26d
Rename BaseProfilerSharedLibraries.h to SharedLibraries.h r=mstange,profiler-reviewers

Comment 10

[Noemi Erli[:noemi_erli]]

Backed out 8 changesets (Bug 1634785) for causing bustages in EHABIStackWalk.cpp CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=3ea11e90d26dd8974b3da41335f5861c491b0c90&searchStr=android%2C5.0%2Carmv7%2Copt

Log: https://treeherder.mozilla.org/logviewer?job_id=474559785&repo=autoland&lineNumber=99688
xpc: https://treeherder.mozilla.org/logviewer?job_id=474563980&repo=autoland&lineNumber=3030

Backout: https://hg.mozilla.org/integration/autoland/rev/72a4810acaeff673dcd4faa17237651e1d5821b7

Comment 11

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Currently Bug 1920956 is blocking this and can't land it until it's fixed.

Comment 12

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1634785 - Apply recent breakpad changes to baseprofiler copy r?mstange

Well, I said recent in the commit title but it's actually 3 years old change.
It was implemented in Bug 1741760. Looks like it was writing garbage code IDs
to the modules that aren't ELF files without this change. So I think it's good
to have it in our copy as well.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b49494dec386
Move SharedLibrary IPC ParamTraits outside of shared library header file r=aabh,mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/dbbb6a6563e8
Add missing shared libraries methods to the baseprofiler implementation r=mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/34ae05420bc4
Make the base profiler shared libraries file closer to gecko profiler one r=mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/c447ddfd70a2
Fix how shared library names are found on linux and macOS r=mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/8524486a6d36
Do not expose AddSharedLibraryFromModuleInfo that's only used internally r=mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/4f17a5b45353
Return early for ntdll.dll if EAF+ is enabled on mozglue SharedLibrary as well r=mstange,profiler-reviewers,bobowen
https://hg.mozilla.org/integration/autoland/rev/9a91de226e6d
Apply recent breakpad changes to baseprofiler copy r=mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/8ad3b7e6de23
Deduplicate shared libraries code r=mstange,profiler-reviewers
https://hg.mozilla.org/integration/autoland/rev/6b399daabb18
Rename BaseProfilerSharedLibraries.h to SharedLibraries.h r=mstange,profiler-reviewers

Comment 14

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/b49494dec386
https://hg.mozilla.org/mozilla-central/rev/dbbb6a6563e8
https://hg.mozilla.org/mozilla-central/rev/34ae05420bc4
https://hg.mozilla.org/mozilla-central/rev/c447ddfd70a2
https://hg.mozilla.org/mozilla-central/rev/8524486a6d36
https://hg.mozilla.org/mozilla-central/rev/4f17a5b45353
https://hg.mozilla.org/mozilla-central/rev/9a91de226e6d
https://hg.mozilla.org/mozilla-central/rev/8ad3b7e6de23
https://hg.mozilla.org/mozilla-central/rev/6b399daabb18