LGTM. I did have one question about the progress bar. Is 5% just a ball park estimate of how much of the start-up process happens before we get to the place where we can count events, or is there a motivating reason behind the number? A short comment explaining why 5% might be helpful. Ideally, would this number be different for users with 1 package, or 1000000 packages installed? Other than that, everything looks good from what I can tell.
Thanks, Brock jmr wrote: > Broc and Michal, > > I have a new spin of the startup webrev. > http://cr.opensolaris.org/~jmr/1723_statup_3087_shutdown_1915_categorysort_v3_Aug29/ > > In addition to making the suggested changes prompted by an idea from > Rich I dug further into getting the GUI to yield and turned up another > approach which does not use generators and seems simpler. > > http://faq.pygtk.org/index.py?req=show&file=faq03.007.htp > > while gtk.events_pending(): > gtk.main_iteration(False) > > Calling it just after the PackageManager is created has the added > advantage of getting the GUI to appear pretty much instantly on my > machine with the progress dialog and everything correctly drawn. > On suggestion from Frank Ludolph I have also added progress bar movement > before loading the package entries. By disabling the filter I keep the > progress_dialog up now until everything has been drawn and the app is > fully functional. > > Goal had been to start app in < 1sec and be functional in < 10 sec on a > modern laptop (2GigHz dual core, 2 Meg Ram). This webrev and Michal's > earlier filter optimization achieves both these goals. > > JR > > Brock Pytlik wrote: > >> John Rice wrote: >> >>> Thanks for taking a look at this Broc, comments below: >>> >>> JR >>> >>> Brock Pytlik wrote: >>> >>>> jmr wrote: >>>> >>>>> Michal - new webrev at: >>>>> >>>>> http://cr.opensolaris.org/~jmr/1723_statup_3087_shutdown_1915_categorysort_v1_Aug27/ >>>>> >>>>> >>>>> >>>>> >>>>> >>>> Lines 1507-1511 make it seem like this is a dead function. Is it a >>>> stub yet to be filled or a function on its way out? Either way, the >>>> comment should reflect that. Also, one space after the # in comments >>>> please. >>>> >>> Not sure which section you are referring to, are you talking about >>> get_installed_version() ? >>> >> Sorry, I think I wrote that looking at a different version and missed >> that line numbers have changed. I was referring to >> >> switch_to_active_valid_image >> >> >>>> Would the separation be even cleaner if __get_image_from_directory >>>> yielded the percentage, or a tuple of pkg_cnt and total_pkg_count >>>> instead of making the call to gobject.idle_add from inside that >>>> function? >>>> >>> I see what you mean we have the setup code in the caller and then >>> manipulate the progress dialog in __get_image_from_directory(). I >>> could do as you suggest or I could pass in the self.w_progress_dialog >>> widget to __get_image_from_directory(), so folks reading the code >>> could see what we were intending. I'll play around with it a bit. >>> >> Either of these sound good. >> [snip] >> >>>> I'm also unclear why you're yielding to a while loop which just >>>> keeps churning through things. A third option would be to have >>>> __get_image_from_directory simple make the calls to >>>> gobject._idle_add both from inside __get_image_from_directory. I'm >>>> not understanding the code structure. I also don't know a great deal >>>> about python GUI programming, so I'd like to understand the >>>> reasoning behind this layout. >>>> >>>> Is what these changes do basically put off starting the package view >>>> until after the graphical shell has been displayed? >>>> >>> That's exactly what we are doing. Otherwise on startup we just get >>> nothing and then a GUI that's a series of gray boxes until the setup >>> functions return, not a very nice startup experience. >>> >>> We tried lots of different approaches here, but the GUI refused to >>> redraw during startup, even when we put the >>> __get_image_from_directory in its own thread, the GUI just wasn't >>> getting the CPU. The gobject.idle_add puts a function on the GUI's >>> task list and the main GTK thread will pick it up when its not doing >>> anything else. With the older structure it never got a chance to >>> service these low priority GUI events. Forcing the >>> __get_image_from_directory function to yield in this way allows us to >>> give control back to the main GTK thread and so we can get the >>> progress updated and all is well. The goal here is not so much about >>> saving time, but clearly showing a user what is being done during >>> startup. I intend to look at optimizing the startup elements next, >>> where it makes sense, particulary in the __application_filter code. >>> >> I think I understand. 'yield' is doing more than just "yield" in the >> generator sense? It's actually yielding in the sense of going to >> sleep/getting of the processor to allow other threads to run? >> >>> Note: We use gobject.idle_add generally when we are in other threads >>> and want to do something which needs to happen on the GTK main loop. >>> See http://faq.pygtk.org/index.py?req=show&file=faq23.020.htp for >>> more details on progress bars in PyGTK and using python generators to >>> create these pseudo-threads. More details on generators and threading >>> in http://www.ibm.com/developerworks/linux/library/l-pythrd.html#author1 >>> >> I haven't had a chance to look at these yet, but I will. >> [snip] >> >> >> Thanks, >> Brock >> > > _______________________________________________ > pkg-discuss mailing list > [email protected] > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
