Tracked down the weirdness. Turned out to be an issue with the progress 
dialog holding on a lock, when we were trying to launch a background 
thread to fetch package descriptions. Changing the sequence and hiding 
the progress dialog so the thread returned before launching the other 
background one sorted it out.

http://cr.opensolaris.org/~jmr/1723_statup_3087_shutdown_1915_categorysort_v9_Sep03

Also seeing a lot of open failures when I was doing some dtracing and 
fixed the ones I could in the packagemanager. Seems a common python 
pattern is to try and do something with a file and catch any exceptions 
if its not there. This can be expensive in a busy function. Better to 
use the os module to check if the file exists first and if its there do 
the required operation. Carved another 0.5 sec off startup for me, so 
I'm happy. Noticed similar pattern in image.py when checking for r+ 
permissions on a file in installed_file_authority(). Could use the 
os.stat module, to get the read status of the file, rather than catching 
exceptions.

JR

jmr wrote:
> Thanks Broc - the 5% is just a ball park stab for some small amount of 
> time that we expect to be pretty constant irrespective of the number of 
> packages.
>
> Having some weirdness when testing this on a few machines, where I'm not 
> getting the packages listed. If I change the ordering to hide the 
> progress dialog after the package entries are loaded but before the list 
> has been displayed by Gtk when attaching the model it seems to work on 
> these other boxes. Not happy about this as the user can then interact 
> with the app before its fully configured. Need to do a little more digging.
>
> JR
>
>
> Brock Pytlik wrote:
>   
>> 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
>   

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to