I understand what you are saying but I'm looking at the output from DTrace python probes on shutdown and get_manifests is being called and adding substantially to the time taken to shutdown. Putting the check in here results in pretty much instance shutdown on my machine.
http://cr.opensolaris.org/~jmr/3087slow_shutdown_v2_Aug27/ JR Michal Pryc wrote: > John, > The subject is about bug 3087, so I belive the webrev is at: > > http://cr.opensolaris.org/~jmr/3087slow_shutdown_v1_Aug27/ > > I am just wondering if this is the best place to put this check. > The performance problem with closing app is because the: > get_manifests_for_packages() > > is running in another thread and it needs to finish download/load one > manifest file before closing app. > > Now you are proposing to return in the function: > self.get_manifest() > > which is being called only from the get_manifests_for_packages() > > so why not to put this check in the get_manifests_for_packages() instead? > > what is the difference: > Currently in the proposal the self.get_manifest() is returning but > get_manifests_for_packages() is still iterating, so simply calling > function which will return. Once this iteration, which I belive is very > fast will finish the app will close. > > If we would put this check in the get_manifests_for_packages() instead > and return from get_manifests_for_packages(), the the iteration will > break and application will not have to call get_manifest(). > > What do you think? > > best > Michal > > jmr wrote: > >> Thanks Michal, respin at: >> >> http://cr.opensolaris.org/~jmr/1723_startup_time_v2_Aug27/ >> >> Made your suggested change for the UnboundLocalError and moved the >> self.application_list_filter.set_visible_func(self.__application_filter) >> down into init_package_view(self), so can't see how this could be called >> now before init is complete. I wasn't seeing the error you reported, >> maybe a timing issue on your machine. >> >> Changed the progress so it does the final call to update the progress on >> completion. >> >> Also pylinted the code, few lines too long and going over number of >> attributes in PackageManager, which are needed, so I modified the >> gui_pylintrc file to allow it. >> >> JR >> >> >> Michal Pryc wrote: >> >>> John, >>> It looks great. Much better user experience :) >>> >>> >>> Two comments: >>> >>> 1. >>> Sometimes I am getting the UnboundLocalError, basically filter is >>> applied before the loop responsible for getting packages finishes. >>> The fix for that would be to change the line 919: >>> >>> 919 if self.preparing_list: >>> 920 return False >>> >>> To: >>> >>> 919 if self.preparing_list and self.in_setup: >>> 920 return False >>> >>> >>> [11:13] --> >>> [EMAIL PROTECTED]:~/Desktop/ips_gui_development/gate_startup_delete_me/src# >>> ./packagemanager.py >>> Traceback (most recent call last): >>> File "./packagemanager.py", line 948, in __application_filter >>> if selected_category == 0: >>> UnboundLocalError: local variable 'selected_category' referenced >>> before assignment >>> Traceback (most recent call last): >>> File "./packagemanager.py", line 948, in __application_filter >>> if selected_category == 0: >>> >>> 2. >>> The setup progress never finishes. It ends at 10192 out of 10435 and >>> then dissappears. The proper behaviour would be to show full 10435 out >>> of 10435 and then hide the progress. >>> >>> best >>> Michal >>> >>> > > _______________________________________________ > pkg-discuss mailing list > pkg-discuss@opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss > _______________________________________________ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
