Store soname as a std::string.

Once upon a time (and, indeed, to this very day if you're on LP32) the
soinfo struct used a fixed-length buffer for the soname. This caused
some issues, mainly with app developers who accidentally included a full
Windows "C:\My Computer\...\libfoo.so" style path. To avoid all this we
switched to just pointing into the ELF file itself, where the DT_SONAME
is already stored as a NUL-terminated string. And all was well for many
years.

Now though, we've seen a bunch of slow startup traces from dogfood where
`dlopen("libnativebridge.so")` in a cold start takes 125-200ms on a recent
device, despite no IO contention. Even though libnativebridge.so is only
20KiB.

Measurement showed that every library whose soname we check required
pulling in a whole page just for the (usually) very short string. Worse,
there's readahead. In one trace we saw 18 pages of libhwui.so pulled
in just for `"libhwui.so\0"`. In fact, there were 3306 pages (~13MiB)
added to the page cache during `dlopen("libnativebridge.so")`. 13MiB for
a 20KiB shared library!

This is the obvious change to use a std::string to copy the sonames
instead. This will dirty slightly more memory, but massively improve
locality.

Testing with the same pathological setup took `dlopen("libnativebridge.so")`
down from 192ms to 819us.

Bug: http://b/177102905
Test: tested with a pathologically modified kernel
Change-Id: I33837f4706adc25f93c6fa6013e8ba970911dfb9

1 files changed, 7 insertions, 12 deletions

diff --git a/linker/linker.cpp b/linker/linker.cpp
index 2481be4e4..77824f615 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -1339,8 +1339,7 @@ static bool find_loaded_library_by_soname(android_namespace_t* ns,
                                           const char* name,
                                           soinfo** candidate) {
   return !ns->soinfo_list().visit([&](soinfo* si) {
-    const char* soname = si->get_soname();
-    if (soname != nullptr && (strcmp(name, soname) == 0)) {
+    if (strcmp(name, si->get_soname()) == 0) {
       *candidate = si;
       return false;
     }
@@ -2571,9 +2570,8 @@ bool VersionTracker::init_verneed(const soinfo* si_from) {
 
     const char* target_soname = si_from->get_string(verneed->vn_file);
     // find it in dependencies
-    soinfo* target_si = si_from->get_children().find_if([&](const soinfo* si) {
-      return si->get_soname() != nullptr && strcmp(si->get_soname(), target_soname) == 0;
-    });
+    soinfo* target_si = si_from->get_children().find_if(
+        [&](const soinfo* si) { return strcmp(si->get_soname(), target_soname) == 0; });
 
     if (target_si == nullptr) {
       DL_ERR("cannot find \"%s\" from verneed[%zd] in DT_NEEDED list for \"%s\"",
@@ -3214,15 +3212,12 @@ bool soinfo::prelink_image() {
   // for apps targeting sdk version < M.) Make an exception for
   // the main executable and linker; they do not need to have dt_soname.
   // TODO: >= O the linker doesn't need this workaround.
-  if (soname_ == nullptr &&
-      this != solist_get_somain() &&
-      (flags_ & FLAG_LINKER) == 0 &&
+  if (soname_.empty() && this != solist_get_somain() && (flags_ & FLAG_LINKER) == 0 &&
       get_application_target_sdk_version() < 23) {
     soname_ = basename(realpath_.c_str());
-    DL_WARN_documented_change(23,
-                              "missing-soname-enforced-for-api-level-23",
-                              "\"%s\" has no DT_SONAME (will use %s instead)",
-                              get_realpath(), soname_);
+    DL_WARN_documented_change(23, "missing-soname-enforced-for-api-level-23",
+                              "\"%s\" has no DT_SONAME (will use %s instead)", get_realpath(),
+                              soname_.c_str());
 
     // Don't call add_dlwarning because a missing DT_SONAME isn't important enough to show in the UI
   }