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. > Great thanks for doing this.
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? 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
