Hi Florian, First of all thank you for your great work, I haven't been involved a lot with yum development lately but just lurking on the mailing-lists because I was busy with some other stuff but I think your depsolve branch shows a lot of potential for both code simplifications and performance increases.
I agree with Seth however that we will need to move towards this goal with small-steps at a time to assure that everybody still understands how everything works and to avoid bugs from appearing in random places. Since I had some spare time tonight, I've taken a look at the first patch. It appears to do a little more than the name suggests. As far as I can tell it does 4 things: 1) Replaces pkgtuples with pkgobjects -> great looks like a rather simple first step required to improve things code quality wise however there appears to be a performance decrease (which can be solved as shown by your Depsolve branch) 2) Removes the missingdep return value from processConflict -> looks sane as it always contains 0 anyway 3) Removes the conflicts return value from processReq -> looks sane as it always contains 0 anyway However there is a fourth step that I can not follow just by reading the patch: It yanks out a lot of stuff from _processReq. Thereby simplifying it a lot, however I see no argumentation why your new (much smaller and simpler code) does exactly the same. Are you sure that all the tests you removed where never being triggered and that there are no corner cases that break because of this? I would suggest splitting the patch into the 4 pieces and implement them one by one. I think the first three parts should be rather easy to get implemented up-stream the last part might require some additional explanation. BTW I performed some testing using a test repo that I created filled with 100 packages with random requirements, provides, etc. And it appears that ffesti with just the first patch applied returns the same results as the current upstream yum (when both are patched with a small patch to always resolve deps alphabetically instead of semi random order returned by calling unique on the array) so there appears to be a valid reason for removing all that code from processReq I just don't see it yet ;-) The full Depsolve patch set returns different results which (in my first analysis) appear to be due to a change in the way that the dependencies are resolved: upstream yum first gathers all requirements for a depsolve phase and then solves them all together while ffesti does it per package in the depsolve phase. This can yield different resolve paths especially when there are multiple choices to resolve a requirement. Greets, Gijs On 8/17/07, Florian Festi <[EMAIL PROTECTED]> wrote: > Hi! > > The patches are still pending. May be people have find testing easier when > the patches are attached... (got rebased to the current upstream) > > > 0001-return-package-objects-instead-of-pkgtuples-from-Dep.patch: > Florian Festi wrote: > > The first patch replaces the NVR tuples returned by the _check*() > > methods by package objects. This allows to kill nearly all code within > > _processReq() which used to reconstruct these package objects. So this > > patch is quite straight forward. > > 0001-kill-_mytscheck-do-global-checks-only-after-all-dep.patch: > > The second reimplements .resolveDeps() while introducing a new > > _resolveRequires() method and kills ._mytscheck(). Reason for this is > > getting ._checkConflicts() and ._checkFilerequires() out of the > > depsolving loop. This significantly speeds up cases with a lot of > > depsolving rounds like my "install [a-k]*" and "remove glibc" test > > cases. The patch also further simplifies the return tuples of the > > .check*() methods. As this patch is a bit less straight forward close > > reviews are welcome. > > Have fun > > Florian > > _______________________________________________ > Yum-devel mailing list > [email protected] > https://lists.dulug.duke.edu/mailman/listinfo/yum-devel > > > _______________________________________________ Yum-devel mailing list [email protected] https://lists.dulug.duke.edu/mailman/listinfo/yum-devel
