Allow heapprofd_standalone_client to build for Chrome#5162
Allow heapprofd_standalone_client to build for Chrome#5162Sentimentron wants to merge 1 commit intogoogle:mainfrom
Conversation
This allows libheaprofd_standalone_client to be included in the chrome_public_apk target for tracing Chrome's custom allocators. * Re-plumbs the GN build to remove/simplify dependencies for Chromium. * Adds a "lib" prefix to kSkipMaps. AI: Utilized Bug: 493606829
|
I'm happy to look at this. I just am a bit delayed due to traveling in CN right now. i'll try to get this later today. |
primiano
left a comment
primiano
left a comment
There was a problem hiding this comment.
Unfortunately, as much as I'm sympathetic with what you are trying to do, this change as proposed is unfeasible because of version skews of libunwindstack. see comments below.
| public_deps = [ "//buildtools:libunwindstack" ] | ||
| if (build_with_chromium && is_android) { | ||
| public_configs = [ "//third_party/libunwindstack:libunwindstack_config" ] | ||
| public_deps = [ "//third_party/libunwindstack" ] |
There was a problem hiding this comment.
so this is likely to cause problems in the rolls.
libunwind stack is subjected to breaking changes every now and then. when it happens, we have to catch up with upstream (Android). But this means whenver we do, the chromium roll will fail.
It's the biggest worry I have about this PR. This is going to doom us to get stuck on the chromium rolls.
I think the best ay forward would be to create an indirection layer in the unwinder, and allow chrome to plug in its unwinder (you can use your own copy of libunwindstack, and that's better because there is no direct coupling)
This dependency is going to be a time-bomb unfortunately
| # This target does absolutely nothing, so we do not want to depend on | ||
| # liblog. | ||
| configs -= [ "//gn/standalone:android_liblog" ] # nogncheck | ||
| if (perfetto_build_standalone) { |
There was a problem hiding this comment.
(here and below) well also `|| build_with_android) no? (or !build_with_chromium to be more conservative)
| sizeof(uint32_t) * unwindstack::X86_REG_LAST, | ||
| sizeof(uint64_t) * unwindstack::X86_64_REG_LAST, | ||
| sizeof(uint64_t) * unwindstack::RISCV64_REG_COUNT}); | ||
| sizeof(uint64_t) * unwindstack::RISCV64_REG_MAX}); |
There was a problem hiding this comment.
this is preceisely the problem I was describing. You are effetively trying to undo https://android-review.git.corp.google.com/c/platform/external/perfetto/+/2918494 likely becuase your libunwindstack version is behind upstream.
Unfortunately this creates a upstream XOR your_version situation that doesn't work for us
|
2 participants
This allows libheaprofd_standalone_client.so to be included in the chrome_public_apk target for tracing Chrome's custom allocators.
Main changes:
kSkipMaps.AI disclosure: I used Gemini CLI to get started and find the main things that needed to change. This PR is derived from that, alongside human review and cleanup.