[pacman-dev] Holy inefficient batman!

[Xavier]

On Sat, Nov 17, 2007 at 07:25:03PM +0100, Xavier wrote:
> And I put a debug back because Aaron asked for it, and I also think it can be
> very handy sometimes, to debug quicker.
> 
> > So I did some speed test, and the results was really suprising:
> > First of all, I kept only deps.c.diff from our patches (my typo doesn't matter 
> > here). And I did "pactest.py -v" on smoke001.py (empty disk cache, repeated 
> > many times times etc.)
> > The result: your patch is _significantly_ slower (I got 5,4,4,4, ... sec) than 
> > mine (I got 4,3,3,3, ... sec). I repeated the test 4 times (patch pacman, 
> > compile, 5 pactest); the result is always the same!
> > Could you test this, too?
> > 
> 
> Hm yes, I'll do more tests.

Well, I get 9.9s with your patch, and 10.8s with mine.
I removed the logging (the 3 lines) in depcmp and I get 10.0s.

But what is one second saved in the runtime of smoke001 compared to the time
this debug log could save you when debugging? :)

Maybe things can still be improved though. What is costly is apparently the
alpm_dep_get_string call. If I remove it, but leave malloc / free and put a
fake depstring, I still get nearly 10.0s.
So I looked at that function for things I could change, it seems it's sprintf
which is not very efficient. I replaced it by manual strncpy (so the code
gets uglier), and I get ~10.0s again.

I'm not sure this is all worth the troubles though.


diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c
index 9232984..31493f7 100644
--- a/lib/libalpm/deps.c
+++ b/lib/libalpm/deps.c
@@ -830,8 +830,8 @@ const char SYMEXPORT *alpm_dep_get_version(const pmdepend_t *dep)
  */
 char SYMEXPORT *alpm_dep_get_string(const pmdepend_t *dep)
 {
-	char *opr, *str = NULL;
-	size_t len;
+	char *opr, *str = NULL, *ptr;
+	size_t namelen, oprlen, verlen;
 
 	ALPM_LOG_FUNC;
 
@@ -858,9 +858,16 @@ char SYMEXPORT *alpm_dep_get_string(const pmdepend_t *dep)
 
 	/* we can always compute len and print the string like this because opr
 	 * and ver will be empty when PM_DEP_MOD_ANY is the depend type */
-	len = strlen(dep->name) + strlen(opr) + strlen(dep->version) + 1;
-	MALLOC(str, len, RET_ERR(PM_ERR_MEMORY, NULL));
-	snprintf(str, len, "%s%s%s", dep->name, opr, dep->version);
+	namelen = strlen(dep->name);
+	oprlen = strlen(opr);
+	verlen =  strlen(dep->version);
+	MALLOC(str, namelen + oprlen + verlen + 1, RET_ERR(PM_ERR_MEMORY, NULL));
+	ptr = str;
+	strncpy(ptr, dep->name, namelen);
+	ptr += namelen;
+	strncpy(ptr, opr, oprlen);
+	ptr += oprlen;
+	strncpy(ptr, dep->version, verlen + 1);
 
 	return(str);
 }