From 14eb4e1decba84cbbaf181ed367f67fc72d88a9e Mon Sep 17 00:00:00 2001 From: Michael Schroeder Date: Thu, 20 Feb 2020 14:43:54 +0100 Subject: [PATCH 1/5] Add fingerprint lookup/compare variants that take ids This will be used to get rid of unneeded id->str->id roundtrips. --- lib/fprint.c | 17 +++++++++++++++++ lib/fprint.h | 17 +++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/lib/fprint.c b/lib/fprint.c index 5cae13b3c3..3dfc2fd714 100644 --- a/lib/fprint.c +++ b/lib/fprint.c @@ -248,6 +248,15 @@ int fpLookup(fingerPrintCache cache, return doLookup(cache, dirName, baseName, *fp); } +int fpLookupId(fingerPrintCache cache, + rpmsid dirNameId, rpmsid baseNameId, + fingerPrint **fp) +{ + if (*fp == NULL) + *fp = xcalloc(1, sizeof(**fp)); + return doLookupId(cache, dirNameId, baseNameId, *fp); +} + /** * Return hash value for a finger print. * Hash based on dev and inode only! @@ -302,6 +311,14 @@ int fpLookupEquals(fingerPrintCache cache, fingerPrint *fp, return FP_EQUAL(*fp, ofp); } +int fpLookupEqualsId(fingerPrintCache cache, fingerPrint *fp, + rpmsid dirNameId, rpmsid baseNameId) +{ + struct fingerPrint_s ofp; + doLookupId(cache, dirNameId, baseNameId, &ofp); + return FP_EQUAL(*fp, ofp); +} + fingerPrint * fpLookupList(fingerPrintCache cache, rpmstrPool pool, rpmsid * dirNames, rpmsid * baseNames, const uint32_t * dirIndexes, diff --git a/lib/fprint.h b/lib/fprint.h index ea3a73b395..3ff945cae2 100644 --- a/lib/fprint.h +++ b/lib/fprint.h @@ -56,6 +56,10 @@ RPM_GNUC_INTERNAL int fpLookupEquals(fingerPrintCache cache, fingerPrint * fp, const char * dirName, const char * baseName); +RPM_GNUC_INTERNAL +int fpLookupEqualsId(fingerPrintCache cache, fingerPrint * fp, + rpmsid dirNameId, rpmsid baseNameId); + RPM_GNUC_INTERNAL const char *fpEntryDir(fingerPrintCache cache, fingerPrint *fp); @@ -75,6 +79,19 @@ int fpLookup(fingerPrintCache cache, const char * dirName, const char * baseName, fingerPrint **fp); +/** + * Return finger print of a file path given as ids. + * @param cache pointer to fingerprint cache + * @param dirNameId id of leading directory name of file path + * @param baseNameId id of base name of file path + * @retval fp pointer of fingerprint struct to fill out + * @return 0 on success + */ +RPM_GNUC_INTERNAL +int fpLookupId(fingerPrintCache cache, + rpmsid dirNameId, rpmsid BaseNameId, + fingerPrint **fp); + /** * Compare two finger print entries. * This routine is exactly equivalent to the FP_EQUAL macro. From 46179a8233c61ef629335e1d327e0674e2a9db0a Mon Sep 17 00:00:00 2001 From: Michael Schroeder Date: Thu, 20 Feb 2020 14:48:56 +0100 Subject: [PATCH 2/5] Get rid of id->str->id roundtrips in rpmalAllFileSatisfiesDepend() We already know the ids, so just use them for fingerprinting. --- lib/rpmal.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rpmal.c b/lib/rpmal.c index ca7ab053ed..be3bd13974 100644 --- a/lib/rpmal.c +++ b/lib/rpmal.c @@ -421,8 +421,8 @@ static rpmte * rpmalAllFileSatisfiesDepend(const rpmal al, const char *fileName, rpmsid dirName = rpmstrPoolIdn(al->pool, fileName, bnStart, 1); if (!al->fpc) - al->fpc = fpCacheCreate(1001, NULL); - fpLookup(al->fpc, rpmstrPoolStr(al->pool, dirName), fileName + bnStart, &fp); + al->fpc = fpCacheCreate(1001, al->pool); + fpLookupId(al->fpc, dirName, baseName, &fp); for (found = i = 0; i < resultCnt; i++) { availablePackage alp = al->list + result[i].pkgNum; @@ -432,7 +432,7 @@ static rpmte * rpmalAllFileSatisfiesDepend(const rpmal al, const char *fileName, if (filterds && rpmteDS(alp->p, rpmdsTagN(filterds)) == filterds) continue; if (result[i].dirName != dirName && - !fpLookupEquals(al->fpc, fp, rpmstrPoolStr(al->pool, result[i].dirName), fileName + bnStart)) + !fpLookupEqualsId(al->fpc, fp, result[i].dirName, baseName)) continue; ret[found] = alp->p; From a92ea2d60a9443d2e2941b5846dd0a5f6a6120d2 Mon Sep 17 00:00:00 2001 From: Michael Schroeder Date: Thu, 20 Feb 2020 14:52:51 +0100 Subject: [PATCH 3/5] Only lookup the fingerprint when the directories do not match If the directories match the fingerprint is not used. --- lib/depends.c | 13 +++++++------ lib/rpmal.c | 17 +++++++++-------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/depends.c b/lib/depends.c index 320d0daf6f..ab8f7bc1c4 100644 --- a/lib/depends.c +++ b/lib/depends.c @@ -881,22 +881,23 @@ static void checkInstFileDeps(rpmts ts, depCache dcache, rpmte te, filedepHashGetEntry(cache, basename, &dirnames, &ndirnames, NULL); if (!ndirnames) return; - if (!fpc) - *fpcp = fpc = fpCacheCreate(1001, NULL); dirname = rpmfiDN(fi); - fpLookup(fpc, dirname, basename, &fp); for (i = 0; i < ndirnames; i++) { char *fpdep = 0; const char *dep; if (!strcmp(dirnames[i], dirname)) { dep = rpmfiFN(fi); - } else if (fpLookupEquals(fpc, fp, dirnames[i], basename)) { + } else { + if (!fpc) + *fpcp = fpc = fpCacheCreate(1001, NULL); + if (!fp) + fpLookup(fpc, dirname, basename, &fp); + if (!fpLookupEquals(fpc, fp, dirnames[i], basename)) + continue; fpdep = rmalloc(strlen(dirnames[i]) + strlen(basename) + 1); strcpy(fpdep, dirnames[i]); strcat(fpdep, basename); dep = fpdep; - } else { - continue; } checkInstDeps(ts, dcache, te, depTag, dep, NULL, is_not); _free(fpdep); diff --git a/lib/rpmal.c b/lib/rpmal.c index be3bd13974..b5190cc9be 100644 --- a/lib/rpmal.c +++ b/lib/rpmal.c @@ -420,10 +420,6 @@ static rpmte * rpmalAllFileSatisfiesDepend(const rpmal al, const char *fileName, fingerPrint * fp = NULL; rpmsid dirName = rpmstrPoolIdn(al->pool, fileName, bnStart, 1); - if (!al->fpc) - al->fpc = fpCacheCreate(1001, al->pool); - fpLookupId(al->fpc, dirName, baseName, &fp); - for (found = i = 0; i < resultCnt; i++) { availablePackage alp = al->list + result[i].pkgNum; if (alp->p == NULL) /* deleted */ @@ -431,10 +427,15 @@ static rpmte * rpmalAllFileSatisfiesDepend(const rpmal al, const char *fileName, /* ignore self-conflicts/obsoletes */ if (filterds && rpmteDS(alp->p, rpmdsTagN(filterds)) == filterds) continue; - if (result[i].dirName != dirName && - !fpLookupEqualsId(al->fpc, fp, result[i].dirName, baseName)) - continue; - + if (result[i].dirName != dirName) { + /* if the directory is different check the fingerprints */ + if (!al->fpc) + al->fpc = fpCacheCreate(1001, al->pool); + if (!fp) + fpLookupId(al->fpc, dirName, baseName, &fp); + if (!fpLookupEqualsId(al->fpc, fp, result[i].dirName, baseName)) + continue; + } ret[found] = alp->p; found++; } From 9c5dd5c2f12862687b81ca9c8775db8fedbd543d Mon Sep 17 00:00:00 2001 From: Michael Schroeder Date: Thu, 20 Feb 2020 15:24:50 +0100 Subject: [PATCH 4/5] Switch dependency checking to use pool ids instead of strings This needs less memory and it should also be faster as the dependencies are already available in id form. --- lib/depends.c | 99 ++++++++++++++++++++++++--------------------------- 1 file changed, 46 insertions(+), 53 deletions(-) diff --git a/lib/depends.c b/lib/depends.c index ab8f7bc1c4..1f8c164e15 100644 --- a/lib/depends.c +++ b/lib/depends.c @@ -51,8 +51,8 @@ const int rpmFLAGS = RPMSENSE_EQUAL; #undef HTDATATYPE #define HASHTYPE filedepHash -#define HTKEYTYPE const char * -#define HTDATATYPE const char * +#define HTKEYTYPE rpmsid +#define HTDATATYPE rpmsid #include "rpmhash.H" #include "rpmhash.C" #undef HASHTYPE @@ -60,7 +60,7 @@ const int rpmFLAGS = RPMSENSE_EQUAL; #undef HTDATATYPE #define HASHTYPE depexistsHash -#define HTKEYTYPE const char * +#define HTKEYTYPE rpmsid #include "lib/rpmhash.H" #include "lib/rpmhash.C" #undef HASHTYPE @@ -870,33 +870,33 @@ static void checkInstFileDeps(rpmts ts, depCache dcache, rpmte te, rpmTag depTag, rpmfi fi, int is_not, filedepHash cache, fingerPrintCache *fpcp) { + rpmstrPool pool = rpmtsPool(ts); fingerPrintCache fpc = *fpcp; fingerPrint * fp = NULL; - const char *basename = rpmfiBN(fi); - const char *dirname; - const char **dirnames = 0; + rpmsid basename = rpmfiBNId(fi); + rpmsid dirname; + rpmsid *dirnames = 0; int ndirnames = 0; int i; filedepHashGetEntry(cache, basename, &dirnames, &ndirnames, NULL); if (!ndirnames) return; - dirname = rpmfiDN(fi); + dirname = rpmfiDNId(fi); for (i = 0; i < ndirnames; i++) { char *fpdep = 0; const char *dep; - if (!strcmp(dirnames[i], dirname)) { + if (dirnames[i] == dirname) { dep = rpmfiFN(fi); } else { if (!fpc) - *fpcp = fpc = fpCacheCreate(1001, NULL); + *fpcp = fpc = fpCacheCreate(1001, pool); if (!fp) - fpLookup(fpc, dirname, basename, &fp); - if (!fpLookupEquals(fpc, fp, dirnames[i], basename)) + fpLookupId(fpc, dirname, basename, &fp); + if (!fpLookupEqualsId(fpc, fp, dirnames[i], basename)) continue; - fpdep = rmalloc(strlen(dirnames[i]) + strlen(basename) + 1); - strcpy(fpdep, dirnames[i]); - strcat(fpdep, basename); + rstrscat(&fpdep, rpmstrPoolStr(pool, dirnames[i]), + rpmstrPoolStr(pool, basename), NULL); dep = fpdep; } checkInstDeps(ts, dcache, te, depTag, dep, NULL, is_not); @@ -905,38 +905,31 @@ static void checkInstFileDeps(rpmts ts, depCache dcache, rpmte te, _free(fp); } -static void addFileDepToHash(filedepHash hash, char *key, size_t keylen) +static void addFileDepToHash(rpmstrPool pool, filedepHash hash, char *key, size_t keylen) { int i; - char *basename, *dirname; + rpmsid basename, dirname; if (!keylen || key[0] != '/') return; for (i = keylen - 1; key[i] != '/'; i--) ; - dirname = rmalloc(i + 2); - memcpy(dirname, key, i + 1); - dirname[i + 1] = 0; - basename = rmalloc(keylen - i); - memcpy(basename, key + i + 1, keylen - i - 1); - basename[keylen - i - 1] = 0; + i++; /* include '/' in dirname */ + dirname = rpmstrPoolIdn(pool, key, i, 1); + basename = rpmstrPoolIdn(pool, key + i, keylen - i, 1); filedepHashAddEntry(hash, basename, dirname); } -static void addDepToHash(depexistsHash hash, char *key, size_t keylen) +static void addDepToHash(rpmstrPool pool, depexistsHash hash, char *key, size_t keylen) { - char *keystr; - if (!keylen) - return; - keystr = rmalloc(keylen + 1); - strncpy(keystr, key, keylen); - keystr[keylen] = 0; - depexistsHashAddEntry(hash, keystr); + if (keylen) + depexistsHashAddEntry(hash, rpmstrPoolIdn(pool, key, keylen, 1)); } static void addIndexToDepHashes(rpmts ts, rpmDbiTag tag, depexistsHash dephash, filedepHash filehash, depexistsHash depnothash, filedepHash filenothash) { + rpmstrPool pool = rpmtsPool(ts); char *key; size_t keylen; rpmdbIndexIterator ii = rpmdbIndexKeyIteratorInit(rpmtsGetRdb(ts), tag); @@ -950,19 +943,29 @@ static void addIndexToDepHashes(rpmts ts, rpmDbiTag tag, key++; keylen--; if (*key == '/' && filenothash) - addFileDepToHash(filenothash, key, keylen); + addFileDepToHash(pool, filenothash, key, keylen); if (depnothash) - addDepToHash(depnothash, key, keylen); + addDepToHash(pool, depnothash, key, keylen); } else { if (*key == '/' && filehash) - addFileDepToHash(filehash, key, keylen); + addFileDepToHash(pool, filehash, key, keylen); if (dephash) - addDepToHash(dephash, key, keylen); + addDepToHash(pool, dephash, key, keylen); } } rpmdbIndexIteratorFree(ii); } +static unsigned int sidHash(rpmsid sid) +{ + return sid; +} + +static int sidCmp(rpmsid a, rpmsid b) +{ + return (a != b); +} + int rpmtsCheck(rpmts ts) { @@ -999,14 +1002,9 @@ int rpmtsCheck(rpmts ts) (depCacheFreeKey)rfree, NULL); /* build hashes of all confilict sdependencies */ - confilehash = filedepHashCreate(257, rstrhash, strcmp, - (filedepHashFreeKey)rfree, - (filedepHashFreeData)rfree); - connothash = depexistsHashCreate(257, rstrhash, strcmp, - (filedepHashFreeKey)rfree); - connotfilehash = filedepHashCreate(257, rstrhash, strcmp, - (filedepHashFreeKey)rfree, - (filedepHashFreeData)rfree); + confilehash = filedepHashCreate(257, sidHash, sidCmp, NULL, NULL); + connothash = depexistsHashCreate(257, sidHash, sidCmp, NULL); + connotfilehash = filedepHashCreate(257, sidHash, sidCmp, NULL, NULL); addIndexToDepHashes(ts, RPMTAG_CONFLICTNAME, NULL, confilehash, connothash, connotfilehash); if (!filedepHashNumKeys(confilehash)) confilehash = filedepHashFree(confilehash); @@ -1016,14 +1014,9 @@ int rpmtsCheck(rpmts ts) connotfilehash = filedepHashFree(connotfilehash); /* build hashes of all requires dependencies */ - reqfilehash = filedepHashCreate(8191, rstrhash, strcmp, - (filedepHashFreeKey)rfree, - (filedepHashFreeData)rfree); - reqnothash = depexistsHashCreate(257, rstrhash, strcmp, - (filedepHashFreeKey)rfree); - reqnotfilehash = filedepHashCreate(257, rstrhash, strcmp, - (filedepHashFreeKey)rfree, - (filedepHashFreeData)rfree); + reqfilehash = filedepHashCreate(8191, sidHash, sidCmp, NULL, NULL); + reqnothash = depexistsHashCreate(257, sidHash, sidCmp, NULL); + reqnotfilehash = filedepHashCreate(257, sidHash, sidCmp, NULL, NULL); addIndexToDepHashes(ts, RPMTAG_REQUIRENAME, NULL, reqfilehash, reqnothash, reqnotfilehash); if (!filedepHashNumKeys(reqfilehash)) reqfilehash = filedepHashFree(reqfilehash); @@ -1053,7 +1046,7 @@ int rpmtsCheck(rpmts ts) /* Check provides against conflicts in installed packages. */ while (rpmdsNext(provides) >= 0) { checkInstDeps(ts, dcache, p, RPMTAG_CONFLICTNAME, NULL, provides, 0); - if (reqnothash && depexistsHashHasEntry(reqnothash, rpmdsN(provides))) + if (reqnothash && depexistsHashHasEntry(reqnothash, rpmdsNId(provides))) checkInstDeps(ts, dcache, p, RPMTAG_REQUIRENAME, NULL, provides, 1); } @@ -1093,13 +1086,13 @@ int rpmtsCheck(rpmts ts) /* Check provides and filenames against installed dependencies. */ while (rpmdsNext(provides) >= 0) { checkInstDeps(ts, dcache, p, RPMTAG_REQUIRENAME, NULL, provides, 0); - if (connothash && depexistsHashHasEntry(connothash, rpmdsN(provides))) + if (connothash && depexistsHashHasEntry(connothash, rpmdsNId(provides))) checkInstDeps(ts, dcache, p, RPMTAG_CONFLICTNAME, NULL, provides, 1); } if (reqfilehash || connotfilehash) { rpmfiles files = rpmteFiles(p); - rpmfi fi = rpmfilesIter(files, RPMFI_ITER_FWD);; + rpmfi fi = rpmfilesIter(files, RPMFI_ITER_FWD); while (rpmfiNext(fi) >= 0) { if (RPMFILE_IS_INSTALLED(rpmfiFState(fi))) { if (reqfilehash) From 4c43d263ee1e01b5f3a9822e8bc24aff17cc0461 Mon Sep 17 00:00:00 2001 From: Michael Schroeder Date: Fri, 21 Feb 2020 16:05:10 +0100 Subject: [PATCH 5/5] Use the transaction set as argument in rpmalCreate() The rpmalCreate() function is called in two places, in both of them the arguments are all read from the transaction set. So simplify the code by passing the ts to rpmalCreate() and getting the values in the constructor. --- lib/depends.c | 7 ++----- lib/rpmal.c | 17 +++++++---------- lib/rpmal.h | 10 +++------- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/lib/depends.c b/lib/depends.c index 1f8c164e15..ede02610cc 100644 --- a/lib/depends.c +++ b/lib/depends.c @@ -384,10 +384,8 @@ rpmal rpmtsCreateAl(rpmts ts, rpmElementTypes types) if (ts) { rpmte p; rpmtsi pi; - rpmstrPool tspool = rpmtsPool(ts); - al = rpmalCreate(tspool, (rpmtsNElements(ts) / 4) + 1, rpmtsFlags(ts), - rpmtsColor(ts), rpmtsPrefColor(ts)); + al = rpmalCreate(ts, (rpmtsNElements(ts) / 4) + 1); pi = rpmtsiInit(ts); while ((p = rpmtsiNext(pi, types))) rpmalAdd(al, p); @@ -455,8 +453,7 @@ static int addPackage(rpmts ts, Header h, } if (tsmem->addedPackages == NULL) { - tsmem->addedPackages = rpmalCreate(rpmtsPool(ts), 5, rpmtsFlags(ts), - tscolor, rpmtsPrefColor(ts)); + tsmem->addedPackages = rpmalCreate(ts, 5); } rpmalAdd(tsmem->addedPackages, p); diff --git a/lib/rpmal.c b/lib/rpmal.c index b5190cc9be..3c8acd63a9 100644 --- a/lib/rpmal.c +++ b/lib/rpmal.c @@ -14,6 +14,7 @@ #include "lib/rpmte_internal.h" #include "lib/rpmds_internal.h" #include "lib/rpmfi_internal.h" +#include "lib/rpmts_internal.h" #include "debug.h" @@ -92,26 +93,22 @@ static void rpmalFreeIndex(rpmal al) al->fpc = fpCacheFree(al->fpc); } -rpmal rpmalCreate(rpmstrPool pool, int delta, rpmtransFlags tsflags, - rpm_color_t tscolor, rpm_color_t prefcolor) +rpmal rpmalCreate(rpmts ts, int delta) { rpmal al = xcalloc(1, sizeof(*al)); - /* transition time safe-guard */ - assert(pool != NULL); - - al->pool = rpmstrPoolLink(pool); + al->pool = rpmstrPoolLink(rpmtsPool(ts)); al->delta = delta; al->size = 0; al->alloced = al->delta; - al->list = xmalloc(sizeof(*al->list) * al->alloced);; + al->list = xmalloc(sizeof(*al->list) * al->alloced); al->providesHash = NULL; al->obsoletesHash = NULL; al->fileHash = NULL; - al->tsflags = tsflags; - al->tscolor = tscolor; - al->prefcolor = prefcolor; + al->tsflags = rpmtsFlags(ts); + al->tscolor = rpmtsColor(ts); + al->prefcolor = rpmtsPrefColor(ts); return al; } diff --git a/lib/rpmal.h b/lib/rpmal.h index 10fe756d43..3a13c5406a 100644 --- a/lib/rpmal.h +++ b/lib/rpmal.h @@ -16,17 +16,13 @@ extern "C" { typedef struct rpmal_s * rpmal; /** - * Initialize available packckages, items, and directory list. - * @param pool shared string pool with base, dir and dependency names + * Initialize available packages, items, and directory list. + * @param ts transaction set * @param delta no. of entries to add on each realloc - * @param tsflags transaction control flags - * @param tscolor transaction color bits - * @param prefcolor preferred color * @return al new available list */ RPM_GNUC_INTERNAL -rpmal rpmalCreate(rpmstrPool pool, int delta, rpmtransFlags tsflags, - rpm_color_t tscolor, rpm_color_t prefcolor); +rpmal rpmalCreate(rpmts ts, int delta); /** * Free available packages, items, and directory members.