Adjust LocalStorage commit rate limiting.

chrome/browser/browser_process_impl.cc

 // Copyright (c) 2012 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/browser/browser_process_impl.h"
 
 #include <algorithm>

[michaeln, 2015/05/15 03:14:33]

I don't buy that almost all areas have uncommitted data. This would be a good
uma stat to gather.

With the existing limits, the avg commit delay time is about 3 mins (no where
near 12 hours). The second part of the change in this CL flushes areas upon the
closing of all pages for a particular origin which should help somewhat.
Yes, that's why i'm keen to wait for domstorage commit tasks here too.
EndSession waits at most 10 seconds, if the couple tasks its currently waiting
on complete more quickly, it can terminate much sooner and then not even try to
store the domstorage data.
Another good stat would be how long it takes to flush domstorage?

With the current impl where the backing store for localstorage is a multiple
sqlitedbs (one per origin), the number of areas to flush may very well be more
significant than the amt of data. Might be more reason to switch to a single
leveldb, to batch flushed changes into a single commit?
I'd vote to plug new numbers into the rate limiter as a first step.

const int kMaxBytesPerHour = kPerStorageAreaQuota;  // longest delay is an hour
const int kMaxCommitsPerHour = 60;  // typical delay less than a minute

 #include <map>
 #include <vector>
 
 #include "base/atomic_ref_count.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/debug/alias.h"
 #include "base/debug/leak_annotations.h"
 #include "base/files/file_path.h"
@@ skipping 387 matching lines @@
 // This is done to ensure that the data has been persisted to disk before
 // continuing.
 class RundownTaskCounter :
     public base::RefCountedThreadSafe<RundownTaskCounter> {
  public:
   RundownTaskCounter();
 
   // Posts a rundown task to |task_runner|, can be invoked an arbitrary number
   // of times before calling TimedWait.
   void Post(base::SequencedTaskRunner* task_runner);
+  void PostMany(
+      const std::vector<scoped_refptr<base::SequencedTaskRunner>> &runners);
 
   // Waits until the count is zero or |max_time| has passed.
   // This can only be called once per instance.
   bool TimedWait(const base::TimeDelta& max_time);
 
  private:
   friend class base::RefCountedThreadSafe<RundownTaskCounter>;
   ~RundownTaskCounter() {}
 
   // Decrements the counter and releases the waitable event on transition to
   // zero.
   void Decrement();
 
   // The count starts at one to defer the possibility of one->zero transitions
   // until TimedWait is called.
   base::AtomicRefCount count_;
   base::WaitableEvent waitable_event_;
 
   DISALLOW_COPY_AND_ASSIGN(RundownTaskCounter);
 };
 
 RundownTaskCounter::RundownTaskCounter()
     : count_(1), waitable_event_(true, false) {
 }
 
+void RundownTaskCounter::PostMany(
+    const std::vector<scoped_refptr<base::SequencedTaskRunner>> &runners) {
+  for (const auto& runner : runners)
+    Post(runner.get());
+}
+
 void RundownTaskCounter::Post(base::SequencedTaskRunner* task_runner) {
   // As the count starts off at one, it should never get to zero unless
   // TimedWait has been called.
   DCHECK(!base::AtomicRefCountIsZero(&count_));
 
   base::AtomicRefCountInc(&count_);
 
   // The task must be non-nestable to guarantee that it runs after all tasks
   // currently scheduled on |task_runner| have completed.
   task_runner->PostNonNestableTask(FROM_HERE,
       base::Bind(&RundownTaskCounter::Decrement, this));
 }
 
 void RundownTaskCounter::Decrement() {
   if (!base::AtomicRefCountDec(&count_))
     waitable_event_.Signal();
 }
 
 bool RundownTaskCounter::TimedWait(const base::TimeDelta& max_time) {
   // Decrement the excess count from the constructor.
   Decrement();
 
   return waitable_event_.TimedWait(max_time);
 }
 
+void FlushStoragePartition(
+    RundownTaskCounter* rundown_counter,
+    content::StoragePartition* partition) {
+  partition->Flush();
+  //rundown_counter->PostMany(partition->GetFlushTaskRunners());
+}
+
 }  // namespace
 
 void BrowserProcessImpl::EndSession() {
   // Mark all the profiles as clean.
   ProfileManager* pm = profile_manager();
   std::vector<Profile*> profiles(pm->GetLoadedProfiles());
   scoped_refptr<RundownTaskCounter> rundown_counter(new RundownTaskCounter());
   for (size_t i = 0; i < profiles.size(); ++i) {
     Profile* profile = profiles[i];
     profile->SetExitType(Profile::EXIT_SESSION_ENDED);
     if (profile->GetPrefs()) {
       profile->GetPrefs()->CommitPendingWrite();
       rundown_counter->Post(profile->GetIOTaskRunner().get());
     }
+    content::BrowserContext::ForEachStoragePartition(
+        profile, base::Bind(FlushStoragePartition, rundown_counter));

[hashimoto, 2015/05/14 04:46:09]

I'm not sure if this will work as expected.
Currently, rundown_counter is responsible to wait for base::ImportantFileWriter
to flush Preferences and Local State.
These tasks are already time consuming. (cf. base::ImportantFileWriter related
items on Profiler page of uma.googleplex.com)

Since it's very likely that almost all DOMStorageArea are having uncommitted
data with the current 12-hour commit interval, it may frequently fail to finish
all tasks before rundown_counter aborts.

Please note that the browser process is explicitly killed in SessionEnding()
after EndSession() returns.
This means if you fail to finish tasks within 10 seconds (=
kEndSessionTimeoutSeconds), data will be lost.

FWIW, I guess here the number of uncommitted data is more important than the
amount of uncommitted data.
Writing DB results in flushing files (e.g. calling fdatasync() on Linux) and it
can take seconds, depending on how busy the disk is.
Since the disk is expected to be extremely busy during OS shutdown, it'll be
more likely to hit the 10 second timeout.

IMO the most straightforward (and the only feasible) solution is removing rate
limiters from DOMStorageArea.

   }
 
   // Tell the metrics service it was cleanly shutdown.
   metrics::MetricsService* metrics = g_browser_process->metrics_service();
   if (metrics && local_state()) {
     metrics->RecordStartOfSessionEnd();
 #if !defined(OS_CHROMEOS)
     // MetricsService lazily writes to prefs, force it to write now.
     // On ChromeOS, chrome gets killed when hangs, so no need to
     // commit metrics::prefs::kStabilitySessionEndCompleted change immediately.
@@ skipping 740 matching lines @@
 }
 
 void BrowserProcessImpl::OnAutoupdateTimer() {
   if (CanAutorestartForUpdate()) {
     DLOG(WARNING) << "Detected update.  Restarting browser.";
     RestartBackgroundInstance();
   }
 }
 
 #endif  // (defined(OS_WIN) || defined(OS_LINUX)) && !defined(OS_CHROMEOS)