19 Nov
2007
19 Nov
'07
11:26 a.m.
> 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);
> }
1. I was curious and I tested smoke001.py with "requiredby-pacman" and as it was
expected by me (no compute-requiredby is needed here), the current version is
much faster (3 or 4 sec vs. 7 sec)
2. There is something in my mind about alpm_dep_getstring:
It shouldn't be used for debug preformat (or in libalpm at all); but that is
good for front-ends.
I would prefer the following more elegant(?) solution instead of switch():
there is a global const char *deptypestr[] = {">=", "<=", "="} in the correct
order somewhere; and from now on, you can pass deptypestr[dep->mod] parameter
easily to alpm_log for example (see alpm_depcmp). This is O(1) (<- switch is
also O(1), but that floods the code imho)
I will create a patch for this, after I can sync my git repo ;-)
Bye
----------------------------------------------------
SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu
This mail sent through IMP: http://horde.org/imp/