simpleperf cache path in latest AOSP uses slashes as separator

[ishitatsuyuki]

I'm trying to use the simpleperf integration with a latest AOSP clone. It seems that the binary cache almost works if I make a change to the cache search path:

diff --git a/wholesym/src/helper.rs b/wholesym/src/helper.rs
index f97e0d5..6eb582e 100644
--- a/wholesym/src/helper.rs
+++ b/wholesym/src/helper.rs
@@ -802,7 +802,7 @@ impl FileAndPathHelper for Helper {
                 let mut build_id_20 = build_id.0.clone();
                 build_id_20.resize(20, 0);
                 let name_in_cache =
-                    format!("{}-{}", ElfBuildId::from_bytes(&build_id_20), binary_name);
+                    format!("{}/{}", ElfBuildId::from_bytes(&build_id_20), binary_name);
                 for dir in &self.config.simpleperf_binary_cache_directories {
                     let p = dir.join(&name_in_cache);
                     paths.push(CandidatePathInfo::SingleFile(
@@ -914,7 +914,7 @@ impl FileAndPathHelper for Helper {
             // Example: binary_cache/5e5c7b9cbc3e65b7c98a139fc1d3e0d000000000-libadreno_utils.so
             let mut build_id_20 = build_id.0.clone();
             build_id_20.resize(20, 0);
-            let name_in_cache = format!("{}-{}", ElfBuildId::from_bytes(&build_id_20), binary_name);
+            let name_in_cache = format!("{}/{}", ElfBuildId::from_bytes(&build_id_20), binary_name);
             for dir in &self.config.simpleperf_binary_cache_directories {
                 let p = dir.join(&name_in_cache);
                 paths.push(CandidatePathInfo::SingleFile(

It looks like the convention was changed in this commit:

https://cs.android.com/android/_/android/platform/system/extras/+/64de86ece6ed6329f017d8408aa19b7755ae3722

I'd like to propose either

1. Updating to use the new convention, or
2. Add both to the search path.

Which one would you prefer?

[mstange]

I'd prefer handling both the old and the new search path. Thanks!

I actually thought I had implemented this long ago, but I guess I hadn't! I think ever since I started respecting the symbol tables in the perf.data file, having access to the files in the binary cache became much less important. The only real use I'm aware of is so that you can see assembly code in the assembly view. Is that what caused you to notice the mismatched paths?

[ishitatsuyuki]

In our use case we are profiling full Android OS builds, where binaries on device are stripped but unstripped versions can be sourced from build directory. So we would be getting much more detailed stack if binary cache search works.

(Also, just wanted to appreciate that the source and assembly views are tremendously useful! The other options available in AOSP are Gecko profile generator or pprof generator, both of which have their pros and limitations, and samply's support for source and assembly is unmatched here.)

[mstange]

In our use case we are profiling full Android OS builds, where binaries on device are stripped but unstripped versions can be sourced from build directory. So we would be getting much more detailed stack if binary cache search works.

How do the unstripped builds get into the binary cache directory? Oh, I see, you can give app_profiler.py / binary_cache_builder.py a -lib argument with the path, I wasn't aware of this. I was using samply import --symbol-dir for this purpose.

(Also, just wanted to appreciate that the source and assembly views are tremendously useful! The other options available in AOSP are Gecko profile generator or pprof generator, both of which have their pros and limitations, and samply's support for source and assembly is unmatched here.)

:)