Places fsync-reduction changes landing tomorrow
Dietrich Ayala
(https://bugzilla.mozilla.org/show_bug.cgi?id=442967)
These changes will be landing with a minor Ts deficit, as the performance and user-experience
benefits of fewer writes were determined in the Firefox meeting to be worth the trade-off. More
details are available here:
https://wiki.mozilla.org/Platform/2008-10-21#Firefox_3.1_Update
The changes will likely land in the early morning PST tomorrow.
-d
sayrer
>
> These changes will be landing with a minor Ts deficit, as the performance and user-experience
> benefits of fewer writes were determined in the Firefox meeting to be worth the trade-off.
I don't think we should land obvious known perf regressions, and I
think the argument of a "trade-off" is silly. All patches are trying
to accomplish something.
It may be the case that we need to take tangential changes to make up
for a regression. That's suboptimal, but requiring it has the distinct
advantage of cutting off arguments from people who are smart enough to
rationalize anything (all of us). It also means our perf numbers might
stay even, but they never go up.
So, fwiw, I'm against this change and the bogus arguments that justify
it.
- Rob
Dietrich Ayala
Drivers and non-drivers at the meeting discussed it, and chose to take a small Ts regression in
exchange for reducing hangs/freezes/sluggishness that occur throughout the entire browsing session
for some users. We're going to continue to spend time working on fixing the regression, but we need
to get the changes into nightlies to get testing and feedback from users who are experiencing the
problem. Why is that unreasonable or bogus?
Sergey Yanovich
> On Oct 21, 4:04 pm, Dietrich Ayala <dietr...@mozilla.com> wrote:
>> These changes will be landing with a minor Ts deficit, as the performance and user-experience
>> benefits of fewer writes were determined in the Firefox meeting to be worth the trade-off.
>
> I don't think we should land obvious known perf regressions, and I
> think the argument of a "trade-off" is silly. All patches are trying
> to accomplish something.
I agree with sayrer that perf regression doesn't seem to be an
accessible option here. IMHO places data isn't so mission critical to
justify fsyncs, but it is still valuable enough to be worth a backup.
I didn't see the proposed patch in the bug 442967 [1] to comment on the
substance. However, the problem discussion [2] doesn't contain a simple
backup solution:
* no fsyncs on places.sqlite while browsing;
* on successful shutdown, copy places.sqlite to places.backup;
* on startup, open places.sqlite. If that fails, copy places.backup to
places.sqlite and repeat.
Pros:
* simple;
* no Ts regression in normal situation.
Cons:
* longer shutdown (but there is no Tshutdown benchmark, right?).
1. https://bugzilla.mozilla.org/show_bug.cgi?id=442967
2. https://wiki.mozilla.org/Places:FsyncApproach
--
Sergey Yanovich
Mike Beltzner
> I agree with sayrer that perf regression doesn't seem to be an
> accessible option here. IMHO places data isn't so mission critical
> to justify fsyncs, but it is still valuable enough to be worth a
> backup.
Is there anyway to fsync on selective activities only? While history
data might be a little less "mission critical", the creation of
bookmarks (or other add-on based annotations, even) should definitely
be considered "mission critical." One of the key rationale for
investing in the Places feature was the transactional security on
bookmarks, fixing several longstanding issues that Firefox and Mozilla
had with random bookmark loss. When a user creates a bookmark, they're
creating an object in their minds that is permanent; if the system
fails hours or even minutes later, a user would (rightfully) expect
that bookmark to be there when the system recovers.
> Cons:
> * longer shutdown (but there is no Tshutdown benchmark, right?).
Why isn't there? There certainly should be, as it's part of
responsiveness overall and does affect the user experience (Trestart =
Tshutdown + Tstart, for example, which is a common operation when
updating one's browser or add-on ... also, Tshutdown affects how
"piggy" an app feels when you decide to dispose of it to end your day,
task, etc.). Not caring about that time because we don't measure it
seems backwards to me.
cheers,
mike
jigarshah
Ts is still acceptable. Currently if i click on any folder in Places
its visibly slow, sometimes really frustrating. I don't have 5 digit
bookmarks but still see slow interface. And for mobile this could be
killing.
But i see in meeting notes.
# This work is not going to make 3.1; push to next release and make
no improvements in this area
# slip beta 2 until we get the performance issues fixed with other
performance improvements
Please..don't do that. allow disabling it if possible but please
include it.
Mike Beltzner
> Please..don't do that. allow disabling it if possible but please
> include it.
While your input is appreciated, I think that the desire for the
feature is well understood, and we're now discussing how to go about
making the decision when faced with the problem that accepting this
code violates a principle we've been sticking to pretty resolutely of
late, which is to not take code known to regress our performance
metrics.
cheers,
mike