Closed Bug 848792 Opened 11 years ago Closed 11 years ago

Dock is significantly increasing CPU usage when showing download progress

Categories

(Toolkit :: Downloads API, defect)

Product:
Toolkit
Component:
Downloads API
Platform:
All
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla23
Tracking Flags:
Tracking Status
firefox22 + fixed
firefox23 --- verified

People

(Reporter: rik, Assigned: dave)

References

Details

(Keywords: perf)

Attachments

(2 files)

Update the dock icon only 8 times per second
737 bytes, patch
jaas
: review+
jaas
: feedback+
akeybl
: approval-mozilla-aurora+
RyanVM
: checkin+
Details | Diff | Splinter Review
Dave's DockProgressTester modified to use only NSDockTile's NSView API
312.73 KB, application/zip
Details 
While downloading a file, my Dock process now takes up to 50% of CPU.

Safari doesn't cause that kind of CPU activity when downloading.
Dave, are you able to look into the cause here?
Can confirm. Actually it does not take 50% of CPU, but does increase the Dock's CPU by over 20% in my tests. One can use this link to test download files:

http://www.thinkbroadband.com/download.html
OS: All → Mac OS X
I can't reproduce this. What version of OS X are you running on?
On nsMacDockSupport.mm:115, we ask for a 30 times per second refresh, which given the size of the progress bar in the dock seems overkill. Maybe just lowering the value to 5 would suffice and be unnoticeable?
(In reply to Dave Vasilevsky from comment #3)
> I can't reproduce this. What version of OS X are you running on?
I'm on 10.8.2.
(In reply to André Reinald from comment #4)
> On nsMacDockSupport.mm:115, we ask for a 30 times per second refresh, which
> given the size of the progress bar in the dock seems overkill. Maybe just
> lowering the value to 5 would suffice and be unnoticeable?

When I had it at a lower value, folks complained it looked choppy. But that's a good backup option if we can't figure out what's wrong.

(In reply to Anthony Ricaud (:rik) from comment #5)
> I'm on 10.8.2.

I unfortunately don't have access to 10.8.
This bug report on VirtualBox sounds like it might be related to 10.8: https://forums.virtualbox.org/viewtopic.php?f=8&t=51624

Could you test with the same profile and build of Firefox in 10.7, to see if it's really 10.8-specific? Maybe you could also try another open-source application with an animated dock icon, and see if it also shows this problem—I think Adium has some animated icon options?

I guess it's possible that +[NSApplication setApplicationIconImage:] has worse performance than using custom drawing in +[NSAppication dockTile], so that may be worth investigating.
Chrome makes the Dock use less than 10% cpu for this. They draw a circle with a flat color. Actually, this might be the issue, we draw an animated bluish bar. It is actually choppy on my machine.

(Not related: They also show the progress bar for the file if it's downloading into a folder displayed into the Dock like Safari)
Keywords: perf
(In reply to Anthony Ricaud (:rik) from comment #8)
> (Not related: They also show the progress bar for the file if it's
> downloading into a folder displayed into the Dock like Safari)
Yes, that is bug 825536.
So I looked at the code for Chromium, just for comparison. They update only 5 times a second, and they also use a dockTile view instead of setApplicationIconImage: . I don't know which one makes more of a difference. If it's the update frequency that's the problem, we'll probably have to change the look of our progress bar so that it doesn't look choppy and awful.  :(

If I put together a test app to see how much of a difference using a dockTile would make, would folks be willing to test it out on 10.8 for me?
Sure, no problem.
For the choppiness of the animation, this is because of bug 848692.
(In reply to Dave Vasilevsky from comment #6)
> (In reply to André Reinald from comment #4)
> > On nsMacDockSupport.mm:115, we ask for a 30 times per second refresh, which
> > given the size of the progress bar in the dock seems overkill. Maybe just
> > lowering the value to 5 would suffice and be unnoticeable?
> 
> When I had it at a lower value, folks complained it looked choppy. But
> that's a good backup option if we can't figure out what's wrong.

We fixed the animation problem in bug 848692. We shouldn't have to update the value of the progress bar that often. Shouldn't the progress bar animate its texture on its own without us refreshing it that often?
fryn, the progress bar in the Dock is not a standard feature, we have to animate it ourselves.
(In reply to Dave Vasilevsky from comment #14)
> fryn, the progress bar in the Dock is not a standard feature, we have to
> animate it ourselves.

Ah, I see. So the 30 times a second isn't just to update the value; it's actually redrawing the progress bar.

(In reply to Dave Vasilevsky from comment #10)
> So I looked at the code for Chromium, just for comparison. They update only
> 5 times a second, and they also use a dockTile view instead of
> setApplicationIconImage: ...

Perhaps that's why they went with a non-animating pie progress bar.

> If I put together a test app to see how much of a difference using a
> dockTile would make, would folks be willing to test it out on 10.8 for me?

I'd be willing to test it, even just as a diff that I can add to my patch queue.
I think the CPU usage here is bad enough that we should back out this feature if a fix is not applied before the next Aurora merge.

I think dockTile is the way to go. Transmission also uses it and that seems recommended in the docs.
https://developer.apple.com/library/mac/#documentation/Carbon/Conceptual/customizing_docktile/docktasks_cocoa/docktasks_cocoa.html#//apple_ref/doc/uid/TP30000986-CH3-SW3
(In reply to Anthony Ricaud (:rik) from comment #8)
> (Not related: They also show the progress bar for the file if it's
> downloading into a folder displayed into the Dock like Safari)

Someone tried to start downloads with both Safari and Chrome simultaneously into the same folder displayed in the Dock?
Dave, do you have time to put together the test app you talked about?
Flags: needinfo?(dave)
Josh, not for the next few days. Is it ok if I do it on the weekend?
Flags: needinfo?(dave)
In that case, we should probably disable/backout the dock animation until we have properly investigated reducing the CPU usage.
I just realized that merge day is next week. I'm looking for a volunteer to back out bug 548763.
status-firefox22: --- → affected
tracking-firefox22: --- → ?
Ok, thanks for your patience folks, I have some time to look at this now.

To start with, I want to get a minimal test case working. I've copied the Firefox dock progress drawing code to a test app: https://github.com/vasi/DockProgressTester . I'd appreciate if folks could build this on 10.8 and report whether or not it shows the same performance issue as folks are seeing in Firefox. 

Also, just to check a suspicion of mine, can you check if it makes a difference whether the Dock is on the side or the bottom of the screen? I'm thinking the CPU usage might have to do with the special effects applied to the Dock when it's on the bottom.


Once we make sure we can reproduce the problem this way, we can look into mitigation. The obvious strategies are:

1. See if using the dockTile API is any better than setApplicationIconImage.
2. See if we can lower the update rate without making animation choppy.
3. See if abandoning phase animation altogether lets us update less often without looking ugly.
4. See if a custom look (like Chrome's circle thing, or a solid blue bar) lets us update even less often.
5. Look into using the NSProgress private API, maybe with fallback to the existing code on 10.6.
Also(In reply to Anthony Ricaud (:rik) from comment #16)
> I think dockTile is the way to go. Transmission also uses it

Transmission also only updates once a second, afaict.
(In reply to Dave Vasilevsky from comment #22)
> Ok, thanks for your patience folks, I have some time to look at this now.
> 
> To start with, I want to get a minimal test case working. I've copied the
> Firefox dock progress drawing code to a test app:
> https://github.com/vasi/DockProgressTester . I'd appreciate if folks could
> build this on 10.8 and report whether or not it shows the same performance
> issue as folks are seeing in Firefox. 
> 
> Also, just to check a suspicion of mine, can you check if it makes a
> difference whether the Dock is on the side or the bottom of the screen? I'm
> thinking the CPU usage might have to do with the special effects applied to
> the Dock when it's on the bottom.
> 
> 
> Once we make sure we can reproduce the problem this way, we can look into
> mitigation. The obvious strategies are:
> 
> 1. See if using the dockTile API is any better than setApplicationIconImage.
> 2. See if we can lower the update rate without making animation choppy.
> 3. See if abandoning phase animation altogether lets us update less often
> without looking ugly.
> 4. See if a custom look (like Chrome's circle thing, or a solid blue bar)
> lets us update even less often.
> 5. Look into using the NSProgress private API, maybe with fallback to the
> existing code on 10.6.

Thanks Dave! Indeed, Dock's CPU is going up by 30% as seen on Firefox. However, you were quite right about Dock position. It would appear that the 3D dock with reflection and Gaussian blur dramatically increases CPU.

Dock on Sides - Goes from .5% -> ~5%

Dock on Bottom - Goes from .5% -> ~35%

If I could suggest something, I would say we follow Safari's approach. If you take a look at their icon, they use a thin, solid blue bar that only updates when progress is made. This almost certainly helps performance. Doing something like that could benefit us and is actually better on OS X integration.
(In reply to Josiah Bruner [:JosiahOne] from comment #25)
> Here's a video showing what Safari does.

Yeah that's the "NSProgress" we discussed in the earlier bug: https://bugzilla.mozilla.org/show_bug.cgi?id=548763#c57

It remains a fallback option if there's nothing better we can do, but I'd still rather find another solution.
(In reply to Dave Vasilevsky from comment #26)
> (In reply to Josiah Bruner [:JosiahOne] from comment #25)
> > Here's a video showing what Safari does.
> 
> Yeah that's the "NSProgress" we discussed in the earlier bug:
> https://bugzilla.mozilla.org/show_bug.cgi?id=548763#c57
> 
> It remains a fallback option if there's nothing better we can do, but I'd
> still rather find another solution.

That's fine. And although UI-review has already been given, I have to say I would prefer if possible that we draw *under* the app icon, instead of on it. At least on Nightly, the progress bar is a little hard to see and looks a little out of place being more squared and longer then the icon.

Either way, that's another issue.

So, if you would like to create another app using the dockTile api, we can compare and see if that helps any. Perhaps it will help keep the Dock from trying to render the blur and reflection. 

setApplicationIconImage is actually modifying the dock icon, so the Dock must keep updating an entire app icon, instead of a small overlay. Although the difference is minimal, perhaps dockTile will keep from the icon continually redrawing itself. Even better would be if dockTile allows the dock to exclude it form the reflection and blur renderings.
(In reply to Josiah Bruner [:JosiahOne] from comment #27)
> I would prefer if possible that we draw *under* the app icon, instead of on
> it.

That's not possible with the public APIs on OS X :(

> At least on Nightly, the progress bar is a little hard to see and looks
> a little out of place being more squared and longer then the icon.

Feel free to tweak the positioning, size and border of the progress bar as it's drawn. Maybe there's a way to make it look nicer.

> So, if you would like to create another app using the dockTile api…

Done! https://github.com/vasi/DockProgressTester/tree/docktile . Testing welcome :)
 
> setApplicationIconImage is actually modifying the dock icon, so the Dock
> must keep updating an entire app icon, instead of a small overlay.

Unfortunately, for our purposes the NSDockTile API also requires drawing the entire dock icon. The only sort of "small overlay" the API allows is -[NSDockTile setBadgeLabel:], which is limited to a string. That's not really acceptable for a progress bar, unless you're a fan of ASCII art :)

I see no performance improvement with NSDockTile, but I'll await testing on 10.8.
I've built DockProgressTester on 10.8.3 and it takes about 5-6% CPU on a Retina MBP, on both vertical and horizontal dock layout. This is far more acceptable than the 30% claimed previously. But it's still a lot, especially battery wise : I've recently fixed bug 839029 for about the same amount of wasted CPU.

For the last few days, I have looked for ways to draw the progress bar in a completely different way, which is much more Cocoa-ish, and never uses low level drawing routines : it's based on NSImageView (to draw the icon of the app), on which we can call addSubview:positioned:relativeTo: to add a NSProgressIndicator (subclass of NSView). Then the "composited" NSImageView is given to the dockTile.

The advantage of NSViews is they handle lots of scaled buffering behind the scenes and, more importantly, they draw "lazily", which means the drawing and compositing are performed at the actual size of the displayed icon, not at higher resolutions.

As quite new to Mozilla, I may be completely wrong here, and maybe you have reasons to use low level drawing APIs. If not, I will implement my approach in the DockProgressTester. Thanks Dave for this little testing app, I feel a lot more comfortable playing with it!
Thanks for popping in, André!

I did look into using NSProgressIndicator, eg in this attachment: https://bug548763.bugzilla.mozilla.org/attachment.cgi?id=480640 (ignore the CIFilter stuff). I abandoned that approach because the indicator didn't want to draw as an active (blue-ish) widget, I needed a weird hack involving a fake NSWindow to get around that. I also found the appearance a little strange, as if it was scaled badly.

But if we can work around those issues, and the CPU usage is better, it could definitely be worth trying. This method would also avoid the hacks I had to do to get the background of the progress bar to look ok with the Carbon calls.
Here's an implementation using NSProgressIndicator: https://github.com/vasi/DockProgressTester/tree/nsprogressindicator . I don't see any improvement in Dock CPU usage. Please test on 10.8.

(Also, this does have the bug I mentioned where the progress bar draws greyed-out, as if it was in the background. It looks like working around that bug may be harder in recent versions of OS X, but I haven't looked into it deeply: http://prod.lists.apple.com/archives/cocoa-dev/2012/Aug/msg00474.html )
Ok, the master branch now allows you to control the update frequency with a slider. The default is 30 updates per second, but you can adjust it lower as the program runs.

I'd appreciate if folks could test it out, especially on 10.8. Try playing with the slider, keeping a close watch on the Dock's CPU usage and on the animation of the progress bar. Try to find a sweet spot where the CPU usage isn't too high, but the animation isn't too choppy.

To make sure you're getting a good look at the animation, you might want to resize the Dock as big as it will go, or zoom in on it with Control + scrollwheel. Remember to look at the subtle background animation on the full part of the bar, especially as it gets very full. That tends to get choppy faster than just the forward progress.

Report back with:
1. The CPU usage you see at 30 updates per second.
2. The CPU usage and number of updates per second that's your "sweet spot"
3. Your impression of how choppy or smooth the animation is at the sweet spot


Here are the numbers I get on my old MacBook running 10.7. My usual setup, with the Dock at the side:

1. 4% CPU at 30 updates per second
2. 4% @ 30 Hz
3. Perfectly smooth

With the Dock on the bottom, maximum size:

1. 15% @ 30 Hz
2. 4% @ 8 Hz
3. Sufficiently smooth, I'd be happy to ship this
On 15" Retina MacBook Pro running 10.8 with Dock on bottom, https://github.com/vasi/DockProgressTester/tree/master shows the following Dock CPU usage:
- 11% @ 6 Hz
- 18% @ 10 Hz
- 21% @ 12 Hz
- 26% @ 15 Hz
- 35% @ 20 Hz
- 53% @ 30 Hz
Predictably linearly proportional, but sure why the usage is so high for me compared to André's results.
10 Hz is enough not to be choppy, especially since the animation moves slowly.
Er, not* sure why.
Okay, so I like where this is headed. I'm not going to give all stats, as here is probably the sweet spot on my machine.

- 6-10% @ 5.2 Hz

Now, 4 Hz is a little choppy for me, but 5 Hz seems fine. However, at this speed CPU% is still not fantastic, but this is on the bottom. That said, an increase of 4-8% is not bad at all and I would be fine shipping this. It will really be up to the reviewer though of how much CPU he/she fines "acceptable".

But for me anything below 10% seems fine.

Don't forget, and I will chance the name of this bug in a second, that CPU percentage will vary depending on the machine, and therefore isn't a great measurement, instead, we should be looking at the increase before the app has started. Also, older CPU with lower speeds may increase their CPU faster.

It may be helpful if any others who test this would list their specs.

E.G, I have a Mac Mini 2011, 2.3GHz Intel Core i5 on a 5400 RPM hard drive. Sine Frank's seem to have been much higher I will ask:

Frank, what specs are you running? What is your CPU% before running the app?
(In reply to Josiah Bruner [:JosiahOne] from comment #35)
> Frank, what specs are you running? What is your CPU% before running the app?

I'm running this on a 2.6GHz Intel Core i7 (quad-core).
I'm using Activity Monitor for these stats, and it's not making any sense to me:
With Firefox, XCode, and Activity Monitor open, Activity Monitor shows the following at the bottom of its window:
% User   approx. 1
% System approx. 1

Then I run DockProgressTester @ 30Hz and it shows the high usage you see in the list in comment 33 and simultaneously the following:
% User   approx. 5
% System approx. 2
Even assuming that % CPU is relative to the maximum usage of 1 CPU and that % User and % System are relative to the total maximum usage of all 4 cores, the math doesn't add up.
(5 + 2) * 4 = 28 < 53
This disparity between total % CPU and % User / % System is also evident for other processes.
Summary: Dock is taking 50% of CPU when showing download progress → Dock is significantly increasing CPU usage when showing download progress
(In reply to Frank Yan (:fryn) from comment #36)
> Even assuming that % CPU is relative to the maximum usage of 1 CPU and that
> % User and % System are relative to the total maximum usage of all 4 cores,
> the math doesn't add up.

Ah, Window -> CPU Usage in Activity Monitor shows eight cores, so I guess each physical core has two virtual "hyper-threaded" cores, so the total CPU capacity is actually 800%.
(In reply to Josh Matthews [:jdm] from comment #21)
> I just realized that merge day is next week. I'm looking for a volunteer to
> back out bug 548763.

And it looks like this didn't happen...

Dave/Dao/Josh - what's your feeling on the likelihood of a low risk fix here in the next couple of weeks? If low, we should nominate a backout for approval-mozilla-aurora.
Assignee: nobody → dave
tracking-firefox22: ? → +
Flags: needinfo?(joshmoz)
Flags: needinfo?(dao)
Judging by folks' reports, I think we can pretty safely just lower the update frequency to, say, 8 times a second. I can throw together a patch in the next couple of days, if someone will commit it.

Does everyone agree that would be an ok solution?
(In reply to Alex Keybl [:akeybl] from comment #38)

> Dave/Dao/Josh - what's your feeling on the likelihood of a low risk fix here
> in the next couple of weeks? If low, we should nominate a backout for
> approval-mozilla-aurora.

I guess that depends on developer availability, happy to review quickly if we have a patch ready.
Flags: needinfo?(joshmoz)
Comment on attachment 732211 [details] [diff] [review]
Update the dock icon only 8 times per second

Josh - can you provide feedback and r+ if this seems like a viable solution?
Attachment #732211 - Flags: feedback?(joshmoz)
Note on my own testing - I get about 20% CPU usage with one window (fedoraproject.org) open and a Fedora DVD download using Firefox 20. With the latest nightly, with dock status, I get 32-35% CPU. This is on a fairly fast MacBook Pro.
Attachment #732211 - Flags: feedback?(joshmoz) → feedback+
Comment on attachment 732211 [details] [diff] [review]
Update the dock icon only 8 times per second

We might be able to do better by picking a less expensive animation and doing it more often, but this seems like a reasonable solution in the mean time.
Attachment #732211 - Flags: review+
I took the DockProgressTester project from Dave and changed it to only use the dockTile "NSView" API. I get less than 3% cpu usage on a MBP retina with 30 refresh per second, whatever the dock orientation, but the progress bar is still grey. I'm currently doing some reverse engineering to find where I can switch it blue. In the meantime reducing the refresh rate is a reasonable solution indeed. If anyone is interested I can post the zip as an attachment.
Thanks for the review, Josh A. I think you may be testing the wrong thing, though. This bug is about the Dock process using a lot of CPU, not the Firefox process.

For this issue, a less expensive animation won't help much, since it's the Dock's fancy blurring effects causing the CPU usage. Here's a test case: https://github.com/vasi/DockProgressTester/blob/cheap . This uses the cheapest progress bar drawing ever, just a blit, two rectangle fills and a rectangle outline. The Dock's CPU usage remains high.

If the Firefox process itself is using a lot of CPU, that's potentially a separate bug. Could you test on exactly the same version and build flags of Firefox, with just the dock progress code disabled? There have been a bunch of changes in downloads UI recently, and I'd want to make sure any regression is actually caused by the dock progress bar and not something else.
André, I'd love to see your code. Could you throw it on github, or post an attachment?

What did you change compared to my test case from comment 31, that made the CPU usage go down? With respect to the greyed-out issue, have you tried putting the NSProgressIndicator inside a fake NSWindow, that pretends to be active?
Here it is. Maybe I missed something. I didn't try the fake NSWindow trick yet. I will default to that if I can't find a cleaner trick to make the progress bar blue.
André, when I test that on 10.7 the Dock process is still using ~15% CPU, same as with the other drawing techniques. Strange that you see a different result, maybe it's a 10.8 thing? Can anybody else test?
On 10.8, André's test app also takes a lot of CPU. So yeah, reducing fps looks like the best way to keep CPU usage low.

Should we open a bug on Apple's bug tracker to notify them of this high CPU usage when Dock is on the bottom?
I didn't realize we were talking about the Dock process, and was focusing on the DockProgressTester process. Sorry for misunderstanding. On 10.8 the Dock process takes about 5% when vertical and 65% when horizontal! We'd better stick with a lower refresh rate as it seems we have no other grasp on Dock's cpu usage.
rik, that's a good idea. If you do, please add it to OpenRadar as well. Feel free to use or adapt DockProgressTester as the test case, I hereby waive all rights to it and release it into the public domain.
I was keeping an eye out for changes in CPU usage in any process, but I didn't notice the dock process changing much. I have a vertical dock though, so it may not have risen enough to get my attention per comment 51.
Attachment #732211 - Flags: checkin?
Attachment #732211 - Flags: checkin? → checkin+
I'm still looking into this performance issue: what if we drop the wave animation of the progress bar (by using a plain color bar instead) so we need only redraw it when its size actually changes? This happens at most at every screen pixel: 128 times for a complete download for a 128 pixel long progress bar (that's the size I get during my tests). We can even approximate to 2 or 3 pixels progress at a time. That should use very little CPU in the end. Any thoughts?
https://hg.mozilla.org/mozilla-central/rev/32001b4712e8
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Comment on attachment 732211 [details] [diff] [review]
Update the dock icon only 8 times per second

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 548763
User impact if declined: High CPU usage of the Dock process while downloading
Testing completed (on m-c, etc.): Yes
Risk to taking this patch (and alternatives if risky): None, just decreasing a timer.
Attachment #732211 - Flags: approval-mozilla-aurora?
Attachment #732211 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(dao)
Keywords: verifyme
In nightly 2013-04-04 the dock CPU was ~30% while downloading, Mac OS X 10.8.3.
In nightly 25.0a1 2013-07-18, FF 23b7 CPU is ~7%.
Calling this verified fixed.
Status: RESOLVED → VERIFIED
status-firefox23: fixed → verified
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: