[ltrace-commits] 09/13: Made activate_latent_in() iterations much more efficient

[Petr Machata]

This is an automated email from the git hooks/post-receive script.

pmachata-guest pushed a commit to branch master
in repository ltrace.

commit 44fb0d5b86b886897b99409a27654a5abf91c9c8
Author: Dima Kogan <dima at secretsauce.net>
Date:   Wed Jul 9 01:09:33 2014 -0700

    Made activate_latent_in() iterations much more efficient
    
    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 | 37 +++++++++----------------------------
 library.h | 25 +++++--------------------
 proc.c    | 42 +++++++++++++++++++-----------------------
 3 files changed, 33 insertions(+), 71 deletions(-)

diff --git a/library.c b/library.c
index aa23262..0c6b483 100644
--- 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;
+}
+
+
 
 
 
diff --git a/library.h b/library.h
index 8e4d059..63b15b7 100644
--- 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
--- 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

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/collab-maint/ltrace.git