Tweak commit interval for ScheduleSavePersistentPrefs.

chrome/common/important_file_writer.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/common/important_file_writer.h"
 
 #include <stdio.h>
 
 #include <string>
 
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
 #include "base/string_number_conversions.h"
 #include "base/task.h"
 #include "base/threading/thread.h"
 #include "base/time.h"
 
 using base::TimeDelta;
 
 namespace {
 
-const int kDefaultCommitIntervalMs = 10000;
+#ifdef NDEBUG

[Paweł Hajdan Jr., 2011/03/18 19:59:03]

I'm not a fan of different behavior in debug vs. release builds. Could you
remove this #if?

[Denis Lagno, 2011/03/21 01:18:53]

I removed dependency on NDEBUG but instead added dependency on OFFICIAL_BUILD or
GOOGLE_CHROME_BUILD

+const int kDefaultCommitIntervalMs = 2000;

[Paweł Hajdan Jr., 2011/03/18 19:59:03]

I'm not sure whether decreasing this interval is the right solution to the
problem. I think we should be either committing everything immediately, or keep
the existing interval.

Another idea would be to make ScheduleWrite call DoScheduledWrite immediately.

Note: even if we post the write task with no delay, it's still executed on
separate thread. If the browser is going to be killed, that doesn't really
"solve" the "data is not saved" problem.

[brettw, 2011/03/18 20:30:21]

If we do keep this different, there should be a comment explaining why.

[Denis Lagno, 2011/03/21 01:18:53]

Done.

[Denis Lagno, 2011/03/21 01:18:53]

original problem I've encountered was the fact that chromeos::RestartJob call
fails to commit pending writes for PrefService.  This problem was resolved by
enforcing committing on this exit path.

I like current implementation with delayed writes.  We keep a lot of stuff in
PrefServices and I am quite sure that there are bursts of writes to them so
commit interval is good optimization.

However I believe that optimal value of commit interval for stable version of
Chrome does not exceed 1-2 seconds.  Values higher than 1-2 seconds give
near-zero gain in performance but they give increasing unsafety.  Probability of
occurence of some unrelated crash grows fast with time passed.  User can do a
real lot in 10 seconds:)

+#else
+const int kDefaultCommitIntervalMs = 100000;
+#endif
 
 class WriteToDiskTask : public Task {
  public:
   WriteToDiskTask(const FilePath& path, const std::string& data)
       : path_(path),
         data_(data) {
   }
 
   virtual void Run() {
     // Write the data to a temp file then rename to avoid data loss if we crash
@@ skipping 105 matching lines @@
   DCHECK(serializer_);
   std::string data;
   if (serializer_->SerializeData(&data)) {
     WriteNow(data);
   } else {
     LOG(WARNING) << "failed to serialize data to be saved in "
                  << path_.value();
   }
   serializer_ = NULL;
 }