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.

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

Reply via email to