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 pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
