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 No worries - will remove it as Michal says its never used. > >>> >>> 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? Exactly - was a little mystified when I started looking at this myself :) The yield allows the GTK to service the functions that have been placed on the idle queue, which update the Progress UI. >> >> 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
