This patch likely needs some explanation too.

rpm-5.0 looks up tag names using a binary, not a linear, search. That
has to be a win, and its on mainline header tag access paths. However,
the effect may be kinda hard to measure objectively, most accesses
just need one or two tags.

The useful parts of the patch has just added a CLI option so that I  
could switch
the binary search on lookup on/off. I'll likely rip the debugging  
next week.

Here's what I see with a --yaml spot check

[jbj@wellfleet wdj]$ rpm -qa --yaml --stats --tagbsearch > /dev/null
    total:               1      0.000000 MB     10.703407 secs
    digest:           1919     33.490485 MB      0.437223 secs
    signature:         611      0.000000 MB      2.363510 secs
    dbget:            1299     34.441996 MB      0.157363 secs
    hdrload:          1290     34.441936 MB      0.172211 secs
    hdrget:          94198      0.000000 MB      0.089633 secs
[jbj@wellfleet wdj]$ rpm -qa --yaml --stats --notagbsearch > /dev/null
    total:               1      0.000000 MB     10.561465 secs
    digest:           1919     33.490485 MB      0.436815 secs
    signature:         611      0.000000 MB      2.359191 secs
    dbget:            1299     34.441996 MB      0.148415 secs
    hdrload:          1290     34.441936 MB      0.164822 secs
    hdrget:          94198      0.000000 MB      0.088836 secs
[jbj@wellfleet wdj]$ rpm -qa --yaml --stats --tagbsearch > /dev/null
    total:               1      0.000000 MB     10.596063 secs
    digest:           1919     33.490485 MB      0.438254 secs
    signature:         611      0.000000 MB      2.347378 secs
    dbget:            1299     34.441996 MB      0.148702 secs
    hdrload:          1290     34.441936 MB      0.164289 secs
    hdrget:          94198      0.000000 MB      0.090396 secs


All that really shows is that --yaml path isn't the right benchmark
for binary <-> linear tag lookup performance improvements. Hmmm ....

Off to remedy the RPM_I18NSTRING_TYPE flaw next. Sometimes
RPM_I18NSTRING_TYPE is an argv, and sometimes its a string, and
its likely related to the testdrive segfault too.

73 de Jeff

On Dec 12, 2007, at 3:01 PM, Jeff Johnson wrote:

>   RPM Package Manager, CVS Repository
>   http://rpm5.org/cvs/
>    
> ______________________________________________________________________ 
> ______
>
>   Server: rpm5.org                         Name:   Jeff Johnson
>   Root:   /v/rpm/cvs                       Email:  jbj@rpm5.org
>   Module: rpm                              Date:   12-Dec-2007  
> 21:01:24
>   Branch: HEAD                             Handle: 2007121220012400
>
>   Modified files:
>     rpm                     CHANGES
>     rpm/lib                 poptALL.c
>     rpm/rpmdb               hdrfmt.c header.c librpmdb.vers
>
>   Log:
>     - attempt to measure performance improvement using binary tag  
> search.
>     - trade an I18NSTRING assert failure for a memory leak for the  
> moment.
>
>   Summary:
>     Revision    Changes     Path
>     1.1978      +2  -0      rpm/CHANGES
>     2.62        +9  -2      rpm/lib/poptALL.c
>     1.57        +4  -1      rpm/rpmdb/hdrfmt.c
>     1.143       +8  -0      rpm/rpmdb/header.c
>     1.40        +1  -0      rpm/rpmdb/librpmdb.vers
>    
> ______________________________________________________________________ 
> ______
>
>   patch -p0 <<'@@ .'
>   Index: rpm/CHANGES
>    
> ====================================================================== 
> ======
>   $ cvs diff -u -r1.1977 -r1.1978 CHANGES
>   --- rpm/CHANGES	12 Dec 2007 19:24:13 -0000	1.1977
>   +++ rpm/CHANGES	12 Dec 2007 20:01:24 -0000	1.1978
>   @@ -1,4 +1,6 @@
>    5.0b1 -> 5.0b2:
>   +    - jbj: attempt to measure performance improvement using  
> binary tag search.
>   +    - jbj: trade an I18NSTRING assert failure for a memory leak  
> for the moment.
>        - jbj: push rpm2cpio w-i-p into cvs, try to measure linear <- 
> > bsearch win.
>        - rpm.org: Logging enhancements (not the best fix, much more  
> needs doing).
>        - rpm.org: Make rpmLogRec opaque.
>   @@ .
>   patch -p0 <<'@@ .'
>   Index: rpm/lib/poptALL.c
>    
> ====================================================================== 
> ======
>   $ cvs diff -u -r2.61 -r2.62 poptALL.c
>   --- rpm/lib/poptALL.c	8 Dec 2007 16:30:49 -0000	2.61
>   +++ rpm/lib/poptALL.c	12 Dec 2007 20:01:24 -0000	2.62
>   @@ -36,6 +36,9 @@
>    extern int _newmagic;
>
>    /*@unchecked@*/
>   +extern int _tagbsearch;
>   +
>   +/*@unchecked@*/
>    extern int _tagcache;
>
>    /*@unchecked@*/
>   @@ -512,10 +515,14 @@
>    	N_("disable rpm signature header"), NULL},
>     { "newmagic", '\0', POPT_ARG_VAL|POPT_ARGFLAG_DOC_HIDDEN,  
> &_newmagic, -1,
>    	N_("dupe region tag into lsb of magic"), NULL},
>   + { "notagbsearch", '\0', POPT_ARG_VAL|POPT_ARGFLAG_DOC_HIDDEN,  
> &_tagbsearch, 0,
>   +	N_("disable tag lookup using binary search"), NULL},
>   + { "tagbsearch", '\0', POPT_ARG_VAL|POPT_ARGFLAG_DOC_HIDDEN,  
> &_tagbsearch, 1,
>   +	N_("enable tag lookup using binary search"), NULL},
>     { "notagcache", '\0', POPT_ARG_VAL|POPT_ARGFLAG_DOC_HIDDEN,  
> &_tagcache, 0,
>   -	N_("disable tag data caching"), NULL},
>   +	N_("disable tag array data caching"), NULL},
>     { "tagcache", '\0', POPT_ARG_VAL|POPT_ARGFLAG_DOC_HIDDEN,  
> &_tagcache, 1,
>   -	N_("enable tag data caching"), NULL},
>   +	N_("enable tag array data caching"), NULL},
>     { "xar", '\0', POPT_ARG_VAL|POPT_ARGFLAG_DOC_HIDDEN, &_use_xar, 1,
>    	N_("read xar package"), NULL},
>
>   @@ .
>   patch -p0 <<'@@ .'
>   Index: rpm/rpmdb/hdrfmt.c
>    
> ====================================================================== 
> ======
>   $ cvs diff -u -r1.56 -r1.57 hdrfmt.c
>   --- rpm/rpmdb/hdrfmt.c	10 Dec 2007 18:37:00 -0000	1.56
>   +++ rpm/rpmdb/hdrfmt.c	12 Dec 2007 20:01:24 -0000	1.57
>   @@ -1792,6 +1792,9 @@
>    /*@unchecked@*/
>    int _tagcache = 1;		/* XXX Cache tag data persistently? */
>
>   +/*@unchechecked@*/
>   +int _tagbsearch = 1;		/* XXX Use bsearch, not linear, tag  
> lookup. */
>   +
>    /** \ingroup header
>     */
>    typedef /*@abstract@*/ struct sprintfTag_s * sprintfTag;
>   @@ -2080,7 +2083,7 @@
>        char *t;
>
>        /* XXX Use bsearch on the "normal" rpmTagTable lookup. */
>   -    if (tbl == NULL || tbl == rpmTagTable) {
>   +    if (_tagbsearch && (tbl == NULL || tbl == rpmTagTable)) {
>    	s = tagName(val);
>    	if (s != NULL && typep != NULL)
>    	    *typep = tagType(val);
>   @@ .
>   patch -p0 <<'@@ .'
>   Index: rpm/rpmdb/header.c
>    
> ====================================================================== 
> ======
>   $ cvs diff -u -r1.142 -r1.143 header.c
>   --- rpm/rpmdb/header.c	9 Dec 2007 16:18:20 -0000	1.142
>   +++ rpm/rpmdb/header.c	12 Dec 2007 20:01:24 -0000	1.143
>   @@ -1894,7 +1894,15 @@
>    	nb = he->c * sizeof(*he->p.ui64p);
>    	break;
>        case RPM_I18NSTRING_TYPE:
>   +#if HACK_AROUND
>    assert(he->c == 1);	/* XXX stop unimplemented oversights. */
>   +#else
>   +	if (he->c > 1) {
>   +	    he->t = RPM_STRING_TYPE;
>   +	    he->p.str = he->p.argv[0];
>   +	    he->c = 1;
>   +	}
>   +#endif
>    	/*@fallthrough@*/
>        case RPM_STRING_TYPE:
>    	if (he->p.str)
>   @@ .
>   patch -p0 <<'@@ .'
>   Index: rpm/rpmdb/librpmdb.vers
>    
> ====================================================================== 
> ======
>   $ cvs diff -u -r1.39 -r1.40 librpmdb.vers
>   --- rpm/rpmdb/librpmdb.vers	4 Dec 2007 22:41:53 -0000	1.39
>   +++ rpm/rpmdb/librpmdb.vers	12 Dec 2007 20:01:24 -0000	1.40
>   @@ -64,6 +64,7 @@
>        _init;
>        _nolead;
>        _newmagic;
>   +    _tagbsearch;
>        _tagcache;
>        _use_xar;
>        prDbiOpenFlags;
>   @@ .
> ______________________________________________________________________
> RPM Package Manager                                    http://rpm5.org
> CVS Sources Repository                                rpm-cvs@rpm5.org