417732 - Delay and HDD activity while zooming (both text- and full-zoom)

Status: VERIFIED FIXED

(Firefox :: General, defect, P3)

Description

[Szabolcs Horvát]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021504 Minefield/3.0b4pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021504 Minefield/3.0b4pre

There is some hard disk activity when increasing/decreasing the zoom factor, and a small, but noticeable delay before the new zoom factor takes effect.  This happens both with full-zoom and text-zoom.  Text-zoom is not as responsive as with Firefox 2.

Reproducible: Always

Steps to Reproduce:
1. Reset zoom factor with CTRL-0.
2. Press CTRL-+ repeatedly and notice that the HDD indicator lights up each time.  Also note the small delay before the zooming happens.  (This might depend on CPU and/or HDD speed, of course.)

Actual Results:  
Text-zoom is noticeably slower than with Firefox 2.

Expected Results:  
Text-zoom should be as fast as with Firefox 2 (instantaneous in my case).

Comment 1

[Elmar Ludwig]

I have not compared the responsiveness with Firefox 2 yet, but I can confirm the hard disk activity here on Linux each time the zoom level changes. From a quick look at an strace dump, it seems like the content pref service is causing the disk activity here.

Comment 2

[Jo Hermans]

The hard disk activity is because Firefox is storing the zoom-factor in the content-prefs.sqlite database, so that it will be remembered the next time you log in. Since we're often zooming in and out a few times, maybe it would be better to delay the saving a few seconds ?

Comment 3

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #2)
> The hard disk activity is because Firefox is storing the zoom-factor in the
> content-prefs.sqlite database, so that it will be remembered the next time you
> log in. Since we're often zooming in and out a few times, maybe it would be
> better to delay the saving a few seconds ?

That's not a bad idea.  We could set a timer when the user changes the zoom, capturing the site for which the timer was set, and then save the setting in the timer callback, restarting the timer if the zoom for that site changes again before it expires.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

My initial assessment was that this shouldn't block, but if it's actually what's causing the slight feeling of sluggishness on zooming (instead of it just taking longer to re-render) then yes, we should make it such that we zoom first, write second. :)

Comment 5

[Szabolcs Horvát]

(In reply to comment #4)
> but if it's actually
> what's causing the slight feeling of sluggishness on zooming (instead of it
> just taking longer to re-render)

Is there a way to turn off saving the zoom factor so that one could test if this is causing the slowdown?  (I'd rather turn it off anyway, because for me it is just an annoyance, especially with text-only zoom.)

Comment 6

[Elmar Ludwig]

Attachment: strace log of "Reset" zoom operation (on Linux)

I was just curious to see how much disk activity is actually going on here, so I did a quick strace log of the zoom operations. The attached file contains most file-related system calls (except for read and seek) that happen during a single "Reset" zoom operation. Some observations:

- "content-prefs.sqlite-journal" is created and deleted three times
- There are six fsync() calls on files opened for writing here, forcing the
  data to be written to disk and waiting until completion. This is likely
  the source of the noticable HDD activity.
- The total amount of data written to disk is about 12 KB.
  (my "content-prefs.sqlite" is 7 KB in size)

Zooming in and out is similar, but "content-prefs.sqlite-journal" is only created two times, this part of the strace log is not attached.

Note: I didn't profile this, so I don't have numbers about how much time is spend where. Zooming in Firefox 2 feels quite a bit faster than zooming in Firefox 3 (trunk, text only zoom), but that may just as well be a general effect of the switch to cairo for drawing.

(In reply to comment #5)
> Is there a way to turn off saving the zoom factor so that one could test if
> this is causing the slowdown?  (I'd rather turn it off anyway, because for
> me it is just an annoyance, especially with text-only zoom.)

Not easily, you would have to edit the browser.js file inside the browser.jar in your firefox installation (at least that is one way to disable it). I just tried that and zooming feels a bit faster without saving the zoom factor on my machine, but it is still slower than Firefox 2.

Comment 7

[Mike Connor [:mconnor]]

Don't think this blocks, there's a lot of factors in the perceived perf slowdown, the HDD activity really shouldn't be it.

Myk, we should look at doing the timer-based solution to avoid excessive writes as a follow-on for v.next.

Comment 8

[Szabolcs Horvát]

(In reply to comment #7)
> Don't think this blocks, there's a lot of factors in the perceived perf
> slowdown, the HDD activity really shouldn't be it.

I would say that it depends on the particular machine, especially on the hard disk speed ...  I recently learned about the preference 'browser.zoom.siteSpecific', and setting it to false clearly improves responsiveness on my laptop (5400 RPM HDD).  Setting this preference to false makes text zoom practically instantaneous, and also prevents the hard-disk activity.  So I believe that at least on my computer the disk activity has the highest impact on the responsiveness of zooming.

Comment 9

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #7)
> Myk, we should look at doing the timer-based solution to avoid excessive writes
> as a follow-on for v.next.

Yup, agreed, we should do that.

The key will be matching async changes to the URIs for which they were made to prevent users from racing the timer when jumping to a new tab after changing the size.

Of course we should also reset the timer each time the user makes a change for the URI and do something smart if the user makes a change for a URI, switches to a different tab, and then makes another change before the timer fires (in that probably-rare edge case we could simply synchronously set the value for the old URI and then reset the timer for the new one).

Comment 10

[Tomasz Sałaciński]

Or maybe the setting should be stored in memory, and saved when user exits Firefox? I've done what mr. Szabolcs Horvát did, and my Firefox 3b5 is about 3x faster when zooming. I know it can be quite hard to implement (especially when everything is done, just need to work in a different way), but I believe users prefer to wait for stable Firefox few days more than killing their hard drive:)

I've noticed that this happens on Windows too.

You can use a small struct array containing only char and int (or float) elements, one for website address, second one for zoom factor. This array can grow in size, when user is browsing, though:(

Comment 11

[u294409]

I can confirm using specified steps. My red HDD led blinks when zooming and the delay is <~ 1sec, not more.

Comment 12

[Bakyt Niyazov]

The hard disk activity is because Firefox is storing the zoom-factor in the
content-prefs.sqlite database, so that it will be remembered the next time you
log in. Since we're often zooming in and out a few times, maybe it would be
better to delay the saving a few seconds ?(In reply to comment #2)
> The hard disk activity is because Firefox is storing the zoom-factor in the
> content-prefs.sqlite database, so that it will be remembered the next time you
> log in. Since we're often zooming in and out a few times, maybe it would be
> better to delay the saving a few seconds ?
> 

I'd prefer Firefox not to memorize current zoom in .sqlite. I think there is no need to store such a data..

Comment 13

[Jo Hermans]

(In reply to comment #12)
> I'd prefer Firefox not to memorize current zoom in .sqlite. I think there is no
> need to store such a data..

It's stored because it's remembered per site, it's not a global setting (in which case it would be a preference). To disable that, see bug 419609 comment 7.

Comment 14

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Re: comment (In reply to comment #7)
> Don't think this blocks, there's a lot of factors in the perceived perf
> slowdown, the HDD activity really shouldn't be it.

It very likely is; I would say that we should go synchronous=OFF for content-prefs.sqlite, since the worst dataloss case there isn't really that bad, but taking N fsyncs is.

Nomming for 1.9.0.1, should be a wafer-thin patch.  (It'll get a bit better with the fix for 421482 anyway, but not enough IMO.)

Comment 15

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: sets PRAGMA SYNCHRONOUS to OFF for content prefs database

This should make things better.

Comment 16

[Karl Tomlinson (:karlt)]

(In reply to comment #15)
> Created an attachment (id=322463) [details]
> patch v1: sets PRAGMA SYNCHRONOUS to OFF for content prefs database

Thanks, Myk.  That should solve the problems seen here.

If the database happens to get corrupted and the corruption is not detected during ContentPrefService__dbInit then it is not clear to me that there will ever be a usable database again.

There are similar issues in bug 434805.

(Perhaps the database will record corruption when detected, so that it can be reported on subsequent open, but I don't know that this happens.)

Is there place that we know will be hit on adding prefs, where another test for corruption could be added?

It may be worth considering if the database is small enough to do an integrity_check on open. (bug 421482 comment 120)

Comment 17

[Mike Connor [:mconnor]]

Comment on attachment 322463 [details] [diff] [review]
patch v1: sets PRAGMA SYNCHRONOUS to OFF for content prefs database

r=me, though the getter for prefsvc may be overkill.

Comment 18

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 322463 [details] [diff] [review]
patch v1: sets PRAGMA SYNCHRONOUS to OFF for content prefs database

a=shaver for immediate trunk landing.

Comment 19

[Myk Melez [:myk] [@mykmelez]]

Checking in toolkit/components/contentprefs/src/nsContentPrefService.js;
/cvsroot/mozilla/toolkit/components/contentprefs/src/nsContentPrefService.js,v  <--  nsContentPrefService.js
new revision: 1.16; previous revision: 1.15
done
Checking in toolkit/components/contentprefs/tests/unit/test_contentPrefs.js;
/cvsroot/mozilla/toolkit/components/contentprefs/tests/unit/test_contentPrefs.js,v  <--  test_contentPrefs.js
new revision: 1.3; previous revision: 1.2
done

Comment 20

[Hasham]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008053008 Firefox/3.0

HDD activity looks to be in check. No delay to report either. RC2 checks out.