Made activate_latent_in() iterations much more efficient
authorDima Kogan <dima@secretsauce.net>
Wed, 9 Jul 2014 08:09:33 +0000 (01:09 -0700)
committerChanho Park <chanho61.park@samsung.com>
Fri, 22 Aug 2014 11:38:26 +0000 (20:38 +0900)
Previously activate_latent_in() iterations looked like

for(export names in lib1) // hash table iteration
{
  for(symbol names in lib2) // list iteration
  {
    if(names equal && libsym->latent)
    {
      proc_activate_latent_symbol(proc, libsym)
    }
  }
}

This is inefficient both due to the double iteration but also since iterating
over a hash table in slow (have to look through all cells, even empty ones).
This patch turns this logic into

for(symbol names in lib2) // list iteration
{
  if(name in lib1 export names && libsym->latent) // hash table lookup
  {
    proc_activate_latent_symbol(proc, libsym)
  }
}

So there's no more double iteration, and the hash iteration was turned into a
hash lookup. Much better.

library.c
library.h
proc.c

index aa23262..0c6b483 100644 (file)
--- a/library.c
+++ b/library.c
@@ -441,34 +441,6 @@ int library_exported_names_push(struct library_exported_names *names,
        return 0;
 }
 
-struct library_exported_names_each_context
-{
-       enum callback_status (*inner_cb)(const char *, void *);
-       void *data;
-};
-static enum callback_status
-library_exported_names_each_cb(const char **key, uint64_t *value, void *data)
-{
-       struct library_exported_names_each_context *context =
-               (struct library_exported_names_each_context*)data;
-       return context->inner_cb(*key, context->data);
-}
-const char** library_exported_names_each(
-       const struct library_exported_names *names,
-       const char **name_start_after,
-       enum callback_status (*cb)(const char *,
-                                  void *),
-       void *data)
-{
-       struct library_exported_names_each_context context =
-               {.inner_cb = cb,
-                .data = data};
-       return DICT_EACH(&names->names,
-                        const char*, uint64_t,
-                        name_start_after, library_exported_names_each_cb,
-                        &context);
-}
-
 struct library_exported_names_each_alias_context
 {
        enum callback_status (*inner_cb)(const char *, void *);
@@ -518,6 +490,15 @@ const char** library_exported_names_each_alias(
                         library_exported_names_each_alias_cb, &context);
 }
 
+int library_exported_names_contains(const struct library_exported_names* names,
+                                   const char* queryname)
+{
+       uint64_t *addr = DICT_FIND_REF(&names->names,
+                                      &queryname, uint64_t);
+       return (addr == NULL) ? 0 : 1;
+}
+
+
 
 
 
index 8e4d059..63b15b7 100644 (file)
--- a/library.h
+++ b/library.h
@@ -262,26 +262,6 @@ int library_exported_names_push(struct library_exported_names *names,
                                uint64_t addr, const char *name,
                                int own_name );
 
-/* Iterates through the a library's export list. The callback is called for
- * every symbol a library exports. Symbol aliases do not apply here. If multiple
- * symbols are defined at the same address, each is reported here.
- *
- * If we want to iterate through the whole list, set name_start_after=NULL. If
- * we want to start iterating immediately past a particular symbol name, pass a
- * pointer to this symbol name in name_start_after. This must be a pointer in
- * the internal dict, preferably returned by an earlier call to this function
- *
- * If the callback fails at any point, a pointer to the failing key is returned.
- * On success, returns NULL. The returned pointer can be passed back to this
- * function in name_start_after to resume skipping this element
- */
-const char** library_exported_names_each(
-       const struct library_exported_names *names,
-       const char **name_start_after,
-       enum callback_status (*cb)(const char *,
-                                  void *),
-       void *data);
-
 /* Iterates through the a library's export list, reporting each symbol that is
  * an alias of the given 'aliasname' symbol. This 'aliasname' symbol itself is
  * NOT reported, so if this symbol is unique, the callback is not called at all.
@@ -304,4 +284,9 @@ const char** library_exported_names_each_alias(
                                   void *),
        void *data);
 
+/* Returns 0 if the exported names list does not contain a given name, or 1 if
+ * it does */
+int library_exported_names_contains(const struct library_exported_names* names,
+                                   const char* queryname);
+
 #endif /* _LIBRARY_H_ */
diff --git a/proc.c b/proc.c
index 394dad0..6c17a88 100644 (file)
--- a/proc.c
+++ b/proc.c
@@ -871,25 +871,18 @@ proc_activate_delayed_symbol(struct process *proc,
 struct activate_latent_in_context
 {
        struct process *proc;
-       struct library *lib;
+       struct library_exported_names *exported_names;
 };
 static enum callback_status
-activate_latent_in_cb(const char *name, void *data)
+activate_latent_in_cb(struct library_symbol *libsym, void *data)
 {
-       const struct activate_latent_in_context *ctx =
-               (const struct activate_latent_in_context*)data;
+       struct activate_latent_in_context *ctx =
+               (struct activate_latent_in_context*)data;
 
-       struct process *proc = ctx->proc;
-       struct library *lib  = ctx->lib;
-
-       struct library_symbol *libsym = NULL;
-       while ((libsym = library_each_symbol(lib, libsym,
-                                            library_symbol_named_cb,
-                                            (void *)name))
-              != NULL)
-               if (libsym->latent
-                   && proc_activate_latent_symbol(proc, libsym) < 0)
-                       return CBS_FAIL;
+       if (libsym->latent &&
+           library_exported_names_contains(ctx->exported_names,
+                                           libsym->name) != 0)
+               proc_activate_latent_symbol(ctx->proc, libsym);
 
        return CBS_CONT;
 }
@@ -897,18 +890,21 @@ activate_latent_in_cb(const char *name, void *data)
 static enum callback_status
 activate_latent_in(struct process *proc, struct library *lib, void *data)
 {
+       struct library_symbol *libsym = NULL;
+
        struct library_exported_names *exported_names =
                (struct library_exported_names*)data;
 
-       struct activate_latent_in_context context = {.proc = proc,
-                                                    .lib = lib};
-       if (library_exported_names_each(exported_names,
-                                       NULL,
-                                       activate_latent_in_cb,
-                                       &context) == NULL)
-               return CBS_CONT;
-       else
+       struct activate_latent_in_context ctx =
+               {.proc = proc,
+                .exported_names = exported_names};
+
+       if (library_each_symbol(lib, libsym,
+                               activate_latent_in_cb,
+                               &ctx) != NULL)
                return CBS_FAIL;
+
+       return CBS_CONT;
 }
 
 void