1400414 - Sync's logging should do less work if we aren't going to write a log

Status: RESOLVED WONTFIX

(Firefox :: Sync, enhancement, P1)

Description

[Thom Chiovoloni [:tcsc] (ex-moco)]

This is related to bug 1350614, but sync specific (since it's easier to handle in the sync case, since we know when our logs are flushed, among other things).

The profile attached to that bug (which is a bit old, but there's no reason to think that much isn't still true) shows that the loggers spend most of their time formatting, which we don't need to do in the cases where logs never are written to disk (which is very common, since we only do this if we hit an error -- unless the user is on nightly or otherwise has turned logOnSuccess to true).

Comment 1

[Thom Chiovoloni [:tcsc] (ex-moco)]

Attachment: Bug 1400414 - Defer formatting work in sync logging until we are sure we actually will write the log.

Review commit: https://reviewboard.mozilla.org/r/180446/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180446/

Comment 2

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8908853 [details]
Bug 1400414 - Defer formatting work in sync logging until we are sure we actually will write the log.

As we discussed in SF, I'm not convinced we actually want this, at least not without some measurements to tell us what kind of win it actually is in practice.

Comment 3

[Thom Chiovoloni [:tcsc] (ex-moco)]

Just spent some time trying to profile this (now that the profile is clearer), and AFAICT the only part involved in logging that actually shows up in profiles of big syncs is actually writing the file (e.g. _copyStreamToFile), which, well, at that point mostly out of options. Closing as wontfix, it looks like it only seemed important due to event loops making the profile more confusing.