Brock Pytlik wrote: > Michal Pryc wrote: >> Brock, >> New webrev is at: >> http://cr.opensolaris.org/~migi/08_12_2008_bug5645_5289_4341_5150/ >> >> I don't know why the pylint didn't catch the nit which you were >> talking about (line 956) but this is corrected. >> >> Also I did clean up the unused static function >> check_if_pkg_have_row_in_model() and removed "\" from the parts which >> I've been changing and contained the "\" inside parens. So a little >> bit more cleanup then the previous webrev. >> Brock, > Great thanks for doing this. Thanks for looking into the changes in the first place!
> I haven't totally groked the changes made and why they offer such a > substantial speed up, but it's clear they do. Thanks for adding the > details to the bug, now at least I can see what's different. > > I'm fine with not moving the code in 1471-1510 into Api.py (or > image.py). I suggest the middle ground of moving it to misc.py and > pulling it out of both client.py and packagemanager.py. While I agree > that robust stable interfaces are desired to solve these problems, I > still don't think we should introduce more potential problems before we > get those interfaces by copying the code. Is that agreeable? That is fine with me. I've made the webrev with this and hopefully you will give me +1 for it, so I will submit to the gate: http://cr.opensolaris.org/~migi/09_12_2008_bug5645_5289_4341_5150/ I did pylint the code as well I've check if everything works fine (pkg list/pkg list -a/pkg list -af/pkg list -afv and packagemanager). I've also ran the tests. -- all the best Michal > Brock >> best >> Michal Pryc >> >> >> Michal Pryc wrote: >>> Brock Pytlik wrote: >>>> General comments: >>>> Could you please update the bugs with the analysis you've done here and >>>> your answers to Padraig's questions? I think that would be helpful. >>> Brock, >>> I have updated all bugs together with some DTrace measuring. I did use >>> my script to limit measured function to __get_image_from_directory(), as >>> this was the main change. The results are attached to bug description. >>> >>>> I'm not quite sure where the speedup is coming from in lines 1470-1505. >>>> Is it because pkgs_known becomes a much smaller list? I'd be interested >>>> in seeing the output of /opt/DTT/Python/py_calltime or py_cputime to >>>> see >>>> where the speed up is happening, and where the continuing bottle necks >>>> are. >>> Please see the results attached to bug: >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5150 >>> >>> We can see that the biggest bottle neck is in the call to the inventory >>> from image.py. >>> >>> Another bottle neck which is not in the output from dtrace as this was >>> not improved is in the call in the image.load_catalogs(), but it is >>> something to address in another thread/bug. >>> >>>> Lines 1470-1505: I'd rather this code wasn't copied, we've been down >>>> that road before. If the gui needs it as well, then I'd suggest >>>> promoting it and placing it in either misc or api.py. Which one, I >>>> don't >>>> really feel strongly about. >>> Before we were using the call: >>> >>> pkgs_known = [ pf[0] for pf in sorted(api_o.img.inventory(all_known = >>> True)) ] >>> >>> I found the sort function from client.py ~1second faster for 22000 fmris >>> that is why I did use it instead of calling sorted(). Also this doesn't >>> need the logic of grouping the packages by calling the: >>> def check_if_pkg_have_row_in_model(pkg, p_pkg) >>> >>> actually I should remove this function from packagemanager.py as well >>> from this webrev as it is not used anymore. >>> >>> I don't think moving this to the api or image at this point is something >>> good as we should start defining the proper functions for getting list >>> of packages and once this will happen the from 1470-1505 code will be >>> gone, but now it is needed to simplify the logic of the function. >>> >>>> nits: >>>> 956: please indent this correctly >>> Will do and propose new webrev. >> > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
