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); }