Comments
-
Greg Hurrell
Thanks for the report.
When Synergy Advance first starts up it reads the iTunes library (XML) file in a separate thread and does as much parsing as possible before returning to the main thread. It does this to keep the UI responsive during the processing.
My library is about a third the size of yours, and I have more CPU and memory so it's difficult to make a comparison. If you have 3200 songs then you you probably have many, many more menu items than you have songs (the exact number depends on how many albums, artists and genres you have). This is because of the way a song can appear in multiple places (such as in the Genres submenu as well as the Artists submenu). See bug #205 in relation to this, which is a request to make these submenus optional or to limit the depth to which they go down. Most likely that request will get implemented for the next release.
Now, there is one point of the operation that cannot occur in the separate thread and has to happen in the main thread, and that is the moment where the menu items that have been prepared are actually inserted into the menu. Even so, I don't think that's the longest part of the operation.
I think the first step to working out what's happening is to add some additional logging so that we can keep track of how long the different phases are taking. I've just added another bug, bug#210, to remind me to add a preference to control the logging level.
So what I'll do is add some additional logging statements in the next release so we can time this and figure out where the bottleneck is.
-
Nigel
Extra logging should be useful. I just launched SA again and noticed it telling me to please wait while it loaded the iTunes database. When I clicked again it had populated the various submenus, but when I clicked again a few seconds later, it gave me the spinning beachball. So it looks like I was wrong with my first report. A) This does happen on all launches, not just the launch upon first program install. B) It might not have to do with loading the database/populating the menus, since it seemed to do that without giving a beachball.
Anyway, I'll hold off on more reports until the next release with more logging, which should hopefully throw some light on this.
-
Greg Hurrell
I'll add logging statements at the following points. At:
1. starting to parse library 2. finished parsing library
3. starting to insert menu items 4. finished inserting menu items
How long does the beachball last? Does it go away if you wait long enough? What does an Activity Monitor sample of the process say when the beachball is happening? (/Applications/Utilities/Activity Monitor -- you can get info on the process and click "Sample" to get a snapshot of where it is spending its time)
Synergy Advance does use some locking to prevent updates to the menu taking place while the insertion is going on. During the lock you should see a beachball but I would hope it would only be a short one. This is necessary because if Synergy Advance is inserting menu items while the menu is down you will see all sorts of ugly tearing and glitches.
-
Nigel
Created an attachment (id=20) Sample of SA while getting spinning beachball
-
Nigel
I attached a sample of the SA application from Activity Monitor. I took the sample while SA was giving me the spinning beachball.
How long does the beachball last?
It depends. After some more observation it seems my last diagnosis was also wrong. It seems it is indeed inserting the menu items which calls SA to beachball. I haven't yet noticed it when it populates the playlist menu. That's understandable as that menu isn't nearly as big as the Genre or Artist menus. The Genre submenu will beachball for probably 5 seconds, and the Artist menu for 10-15 seconds. I haven't timed them, so I could be wrong, but you can get an idea of the length of time SA is unresponsive for.
So it seems like SA parses the iTunes database, then creates the Playlist, Genre, and Artist submenus and inserts them one by one. By clicking during different points of the startup process, I was able to see the global menu before the Genre submenu was created, get the spinning beachball while the Genre submenu was created, then view the Genre submenu before the Artist submenu was created, then get the beachball while the Artist submenu was created, then finally view all the submenus.
Does it go away if you wait long enough?
Yes, it goes away after it finishes creating the submenus.
If this is an issue at the OS level, where creating such large menus simply takes a long time, there may not be much you can do about it (apart from possibly giving some type of indication that the app hasn't frozen).
-
Greg Hurrell
Thanks a lot, Nigel. You've done some excellent research.
You are exactly correct that it inserts the submenus one at a time. It does so to avoid putting a great big lock around the entire operation (which would make the entire global menu unavailable for one long time); instead it uses three smaller locks (making it unavailable for three shorter periods of time instead).
We now have a fairly good idea of where the app is spending its time. We'll have an even better idea when you try the next preview (with the extra logging). Sounds sensible what you suggest about the amount of time each menu takes to be added. In my case I only have a few playlists (about a dozen), about 18 genres, and 195 artists. So the artists submenu is by far the most time-consuming one.
You're right that there's little that can be done to speed up how fast the operating system adds items to a menu (specifically we are talking about the speed of the NSMenu "addItem" method). I can make small optimizations around it but they probably won't affect the overall speed very much.
On thing I may be able to do is user even finer-grained locking to minimize the beachball. It should be possible for me to use a lock that doesn't operate at the level of the global menu as a whole, but instead at the level of the individual submenus. If this is the case then you'd be able to use the global menu when the items were being inserted but you would not be able to access the submenus. Fingers crossed, I think this will be the way to go. If you try to access one of those submenus while the insertion is taking place you'll still get a beachball, but I might be able to avoid even that little beachball by appropriately ghosting the menu item before beginning the insertion.
(The other alternative is to avoid the beachball entirely by only showing the global menu when the whole thing is done. I don't really like that idea though.)
Thanks a lot for bringing this to my attention. It's a real help in getting a nice high polish-factor on the Global Menu.
-
Nigel
Fingers crossed, I think this will be the way to go. If you try to access one
of those submenus while the insertion is taking place you'll still get a beachball, but I might be able to avoid even that little beachball by appropriately ghosting the menu item before beginning the insertion.
If possible, that sounds like a great idea.
-
Greg Hurrell
Ok, I've implemented finer grained locking so I am hoping the problem is now fixed. I really can't do any fewer statements inside the lock now. Hopefully when you try the next preview you'll be able to confirm for me that there's no beachballing at all now.
-
Greg Hurrell
Ok. I've tested this as heavily as I can and there's nothing I can do to make this beachball now that I have implemented the finer grained locking. Those submenus are now ghosted during the update too so that you should never run into a lock contention situation.
I am going to add the fixed-in-prerelease keyword to this bug. Let me know if you want me to email you a build for you to try out, Nigel.
-
Nigel
Let me know if you want me to email you a build for you to try out, Nigel.
Sure, why not. I'm a bit swamped at the moment so might not get to it for a little while, but if I have time I'd be happy to try it out. Sounds like you've come up with a good solution.
-
Nigel
Created an attachment (id=21) SA sample while hanging
I'd like to say the new finer-grained locking and menu-greying out solved the problems of the spinning beachball, but something seems to have gone wrong. I started up SA and saw the greyed out menu items. The Playlist submenu appeared without a problem, and I kept clicking the global menu to watch for the Genre submenu appear. However, before it did SA hung and gave me the spinning beachball. I took a sample of the process with Activity Monitor and attached it here. It's been about 10 minutes and it's still hung, so there seems to be some kind of critical bug.
I don't have time to investigate further right now (heh, I should be working, not testing SA), but hopefully the sample will help a bit. Sorry to be the bearer of bad news.
-
Greg Hurrell
Well, it's waiting for a lock and never getting it. I guess we'll know which one when I release the next preview with the logging options in the advanced preferences.
-
Greg Hurrell
Ok, I've now implemented the logging preference (request #210) and I've added log statements to track down the locking issue. Whenever the log level is set to 5 or higher the locking debugging info will get printed to the console.
I am fairly optimistic about nailing this problem because I can now reproduce it here fairly reliably.
-
Greg Hurrell
Ok, this is going to be interesting.... with logging turned *on* I cannot reproduce the hanging, ever (have tried many times).... With logging turned *off* I can reproduce it at least 33% of the time... So evidently there is some kind of weird threading interaction going on here where the act of logging might be enforcing some kind of synchronization between threads which suppresses the hangs... Sigh...
-
Greg Hurrell
Ok, I think I've now found and fixed the problem. Basically I had something vaguely like this:
1. Worker thread: a) get lock (will wait for ever if need be, but that's not a problem seeing as this is a background thread). b) update menu on main thread.
2. Main thread: a) when user clicks menu, get lock (which means waiting until any in-progress updates are completed).
If the worker thread gets the lock (1.a), and *then* the user tries to click the menu (2.a) but *before* the menu update is done (1.b), we'll get a hang. The main thread will wait forever for the lock; the problem is that the worker thread won't relinquish the lock until the main thread has finished updating the menu, and that will never happen because it's waiting for the lock. Now I mistakenly assumed that that could never happen; ie. I thought that due to the nature of the runloop that (2.a) could never occur before (1.b) and that (1.a) and (1.b) would always execute atomically. Evidently this is not the case, and in fact it seems to happen all too often, but seeing as Cocoa is not open source I have no way of knowing exactly what it's trying to do internally. One of the joys of multithreaded programming.
I think the logging statements must have some kind of weird interaction with the runloop and the performSelectorOnMainThread:withObject:waitUntilDone: method, because when logging was turned on, the hangs weren't happening.
So I've moved the menu updates into the worker thread and removed the calls to performSelectorOnMainThread:withObject:waitUntilDone: method. I didn't want to do this because the Cocoa thread safety docs do not say anything about the thread safety (or otherwise) of NSMenu and NSMenuItem, and the conservative approach would be to assume that if it doesn't explicitly say that something is threadsafe then it is most likely not threadsafe and one should do all updates to the UI from within the main thread.
Even so, I don't think the move is too risky; the use of locks here means that only one thread at a time should be updating the menu items, and never while the menu is tracking. If we discover any cases where this is not the case (ie. the app crashes when clicking the menu) then I'll have to look at the code again. But these changes should prevent the hangs, and unless there are bugs in my code the changes shouldn't cause any crashes.
Adding the fixed-in-prelease keyword again. Fingers crossed.
-
Greg Hurrell
The 0.2 public preview release is now out on the servers.
Add a comment
Comments are now closed for this issue.