| Message ID | 20200130052710.201469-1-anatol.pomozov@gmail.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [pacman-dev] Eliminate extra loop over dbs_sync | expand |
On 30/1/20 3:27 pm, Anatol Pomozov wrote: > Current flow looks like > loop dbs_sync { > loop pkgs { > if pkg.db == db then process(pkg, db) > } > } > > Package sync transaction always has a counterpart in the dbs_sync list > (I cannot come up with a use-case when it is not true). So the loop can > be simplified to: > > loop pkgs { > process(pkg, pkg.db) > } > > Tested: 'ninja test' & manually by using pacman with this patch for a > week > Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com> I have been trying to figure out if there was an historical reason for this... The loops was added in: d37ad0487 (Aaron Griffin 2006-10-15 19:31:03 +0000 805) which is a big merge from Frugalware. So in short, this looks fine to me. Allan
On 31/1/20 12:37 am, Allan McRae wrote: > On 30/1/20 3:27 pm, Anatol Pomozov wrote: >> Current flow looks like >> loop dbs_sync { >> loop pkgs { >> if pkg.db == db then process(pkg, db) >> } >> } >> >> Package sync transaction always has a counterpart in the dbs_sync list >> (I cannot come up with a use-case when it is not true). So the loop can >> be simplified to: >> >> loop pkgs { >> process(pkg, pkg.db) >> } >> >> Tested: 'ninja test' & manually by using pacman with this patch for a >> week >> Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com> > > I have been trying to figure out if there was an historical reason for > this... The loops was added in: > > d37ad0487 (Aaron Griffin 2006-10-15 19:31:03 +0000 805) > > which is a big merge from Frugalware. > > So in short, this looks fine to me. I haven't confirmed... but it likely effects download order. Currently downloads occur roughly in repo order. I don't think it matters, but thought I would flag it anyway. Allan
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index d02a435f..4d93f41f 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -705,16 +705,14 @@ static struct dload_payload *build_payload(alpm_handle_t *handle, return payload; } -static int find_dl_candidates(alpm_db_t *repo, alpm_list_t **files) +static int find_dl_candidates(alpm_handle_t *handle, alpm_list_t **files) { - alpm_list_t *i; - alpm_handle_t *handle = repo->handle; - - for(i = handle->trans->add; i; i = i->next) { + for(alpm_list_t *i = handle->trans->add; i; i = i->next) { alpm_pkg_t *spkg = i->data; - if(spkg->origin != ALPM_PKG_FROM_FILE && repo == spkg->origin_data.db) { + if(spkg->origin != ALPM_PKG_FROM_FILE) { char *fpath = NULL; + alpm_db_t *repo = spkg->origin_data.db; if(!repo->servers) { handle->pm_errno = ALPM_ERR_SERVER_NONE; @@ -802,9 +800,7 @@ static int download_files(alpm_handle_t *handle) handle->totaldlcb(total_size); } - for(i = handle->dbs_sync; i; i = i->next) { - errors += find_dl_candidates(i->data, &files); - } + errors += find_dl_candidates(handle, &files); if(files) { /* check for necessary disk space for download */
Current flow looks like loop dbs_sync { loop pkgs { if pkg.db == db then process(pkg, db) } } Package sync transaction always has a counterpart in the dbs_sync list (I cannot come up with a use-case when it is not true). So the loop can be simplified to: loop pkgs { process(pkg, pkg.db) } Tested: 'ninja test' & manually by using pacman with this patch for a week Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com> --- lib/libalpm/sync.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)