Disk cache: Extend the internal buffering performed by each entry

to cover external files.

We now keep a variable-size buffer and use it even after we
know that the data is not going to be stored by a block-file.
The backend keeps track of the total memory used by all entries
and prevents that value from going over a max value that
depends on the total memory available.

This CL removes the tests that were checking the synchronous
operation of sparse IO because that model is no longer supported
by the public API, and this CL would add complexity to them
(they fail due to thread safety concerns).

BUG=6626
TEST=net_unittests

Review URL: http://codereview.chromium.org/3167020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57082 0039d316-1c4b-4281-b951-d872f2087c98

net/disk_cache/backend_impl.cc

@@ -465,6 +465,7 @@ int BackendImpl::SyncInit() {
   entry_count_ = byte_count_ = 0;
 
   if (!restarted_) {
+    buffer_bytes_ = 0;
     trace_object_ = TraceObject::GetTraceObject();
     // Create a recurrent timer of 30 secs.
     int timer_delay = unit_test_ ? 1000 : 30000;
@@ -968,6 +969,22 @@ void BackendImpl::TooMuchStorageRequested(int32 size) {
   stats_.ModifyStorageStats(0, size);
 }
 
+bool BackendImpl::IsAllocAllowed(int current_size, int new_size) {
+  DCHECK_GT(new_size, current_size);
+  int to_add = new_size - current_size;
+  if (buffer_bytes_ + to_add > MaxBuffersSize())
+    return false;
+
+  buffer_bytes_ += to_add;
+  HISTOGRAM_COUNTS("DiskCache.BufferBytes", buffer_bytes_ / 10);
+  return true;
+}
+
+void BackendImpl::BufferDeleted(int size) {
+  buffer_bytes_ -= size;
+  DCHECK_GE(size, 0);
+}
+
 bool BackendImpl::IsLoaded() const {
   CACHE_UMA(COUNTS, "PendingIO", GetSizeGroup(), num_pending_io_);
   if (user_flags_ & kNoLoadProtection)
@@ -1865,4 +1882,22 @@ bool BackendImpl::CheckEntry(EntryImpl* cache_entry) {
   return !rankings->dummy;
 }
 
+int BackendImpl::MaxBuffersSize() {
+  static int64 total_memory = base::SysInfo::AmountOfPhysicalMemory();
+  static bool done = false;
+
+  if (!done) {
+    const int kMaxBuffersSize = 30 * 1024 * 1024;
+
+    // We want to use up to 2% of the computer's memory.
+    total_memory = total_memory * 2 / 100;
+    if (total_memory > kMaxBuffersSize || total_memory <= 0)
+      total_memory = kMaxBuffersSize;
+
+    done = true;
+  }
+
+  return static_cast<int>(total_memory);
+}
+
 }  // namespace disk_cache

net/disk_cache/backend_impl.h

@@ -175,6 +175,17 @@ class BackendImpl : public Backend {
   // Logs requests that are denied due to being too big.
   void TooMuchStorageRequested(int32 size);
 
+  // Returns true if a temporary buffer is allowed to be extended.
+  bool IsAllocAllowed(int current_size, int new_size);
+
+  // Tracks the release of |size| bytes by an entry buffer.
+  void BufferDeleted(int size);
+
+  // Only intended for testing the two previous methods.
+  int GetTotalBuffersSize() const {
+    return buffer_bytes_;
+  }
+
   // Returns true if this instance seems to be under heavy load.
   bool IsLoaded() const;
 
@@ -314,6 +325,9 @@ class BackendImpl : public Backend {
   // Part of the self test. Returns false if the entry is corrupt.
   bool CheckEntry(EntryImpl* cache_entry);
 
+  // Returns the maximum total memory for the memory buffers.
+  int MaxBuffersSize();
+
   InFlightBackendIO background_queue_;  // The controller of pending operations.
   scoped_refptr<MappedFile> index_;  // The main cache index.
   FilePath path_;  // Path to the folder used as backing storage.
@@ -329,6 +343,7 @@ class BackendImpl : public Backend {
   int num_pending_io_;  // Number of pending IO operations.
   int entry_count_;  // Number of entries accessed lately.
   int byte_count_;  // Number of bytes read/written lately.
+  int buffer_bytes_;  // Total size of the temporary entries' buffers.
   net::CacheType cache_type_;
   int uma_report_;  // Controls transmision of UMA data.
   uint32 user_flags_;  // Flags set by the user.

net/disk_cache/disk_cache_test_base.cc

@@ -4,6 +4,7 @@
 
 #include "net/disk_cache/disk_cache_test_base.h"
 
+#include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
 #include "net/disk_cache/backend_impl.h"
@@ -190,3 +191,35 @@ void DiskCacheTestWithCache::FlushQueueForTest() {
   int rv = cache_impl_->FlushQueueForTest(&cb);
   EXPECT_EQ(net::OK, cb.GetResult(rv));
 }
+
+int DiskCacheTestWithCache::ReadData(disk_cache::Entry* entry, int index,
+                                     int offset, net::IOBuffer* buf, int len) {
+  TestCompletionCallback cb;
+  int rv = entry->ReadData(index, offset, buf, len, &cb);
+  return cb.GetResult(rv);
+}
+
+
+int DiskCacheTestWithCache::WriteData(disk_cache::Entry* entry, int index,
+                                      int offset, net::IOBuffer* buf, int len,
+                                      bool truncate) {
+  TestCompletionCallback cb;
+  int rv = entry->WriteData(index, offset, buf, len, &cb, truncate);
+  return cb.GetResult(rv);
+}
+
+int DiskCacheTestWithCache::ReadSparseData(disk_cache::Entry* entry,
+                                           int64 offset, net::IOBuffer* buf,
+                                           int len) {
+  TestCompletionCallback cb;
+  int rv = entry->ReadSparseData(offset, buf, len, &cb);
+  return cb.GetResult(rv);
+}
+
+int DiskCacheTestWithCache::WriteSparseData(disk_cache::Entry* entry,
+                                            int64 offset,
+                                            net::IOBuffer* buf, int len) {
+  TestCompletionCallback cb;
+  int rv = entry->WriteSparseData(offset, buf, len, &cb);
+  return cb.GetResult(rv);
+}

net/disk_cache/disk_cache_test_base.h

@@ -13,6 +13,12 @@
 
 class FilePath;
 
+namespace net {
+
+class IOBuffer;
+
+}  // namespace net
+
 namespace disk_cache {
 
 class Backend;
@@ -90,6 +96,14 @@ class DiskCacheTestWithCache : public DiskCacheTest {
   int DoomEntriesSince(const base::Time initial_time);
   int OpenNextEntry(void** iter, disk_cache::Entry** next_entry);
   void FlushQueueForTest();
+  int ReadData(disk_cache::Entry* entry, int index, int offset,
+                net::IOBuffer* buf, int len);
+  int WriteData(disk_cache::Entry* entry, int index, int offset,
+                net::IOBuffer* buf, int len, bool truncate);
+  int ReadSparseData(disk_cache::Entry* entry, int64 offset, net::IOBuffer* buf,
+                     int len);
+  int WriteSparseData(disk_cache::Entry* entry, int64 offset,
+                      net::IOBuffer* buf, int len);
 
   // cache_ will always have a valid object, regardless of how the cache was
   // initialized. The implementation pointers can be NULL.

net/disk_cache/entry_impl.h

@@ -133,6 +133,7 @@ class EntryImpl : public Entry, public base::RefCounted<EntryImpl> {
   enum {
      kNumStreams = 3
   };
+  class UserBuffer;
 
   ~EntryImpl();
 
@@ -158,17 +159,28 @@ class EntryImpl : public Entry, public base::RefCounted<EntryImpl> {
   // given offset.
   bool PrepareTarget(int index, int offset, int buf_len, bool truncate);
 
-  // Grows the size of the storage used to store user data, if needed.
-  bool GrowUserBuffer(int index, int offset, int buf_len, bool truncate);
+  // Adjusts the internal buffer and file handle for a write that truncates this
+  // stream.
+  bool HandleTruncation(int index, int offset, int buf_len);
+
+  // Copies data from disk to the internal buffer.
+  bool CopyToLocalBuffer(int index);
 
   // Reads from a block data file to this object's memory buffer.
   bool MoveToLocalBuffer(int index);
 
   // Loads the external file to this object's memory buffer.
-  bool ImportSeparateFile(int index, int offset, int buf_len);
+  bool ImportSeparateFile(int index, int new_size);
+
+  // Makes sure that the internal buffer can handle the a write of |buf_len|
+  // bytes to |offset|.
+  bool PrepareBuffer(int index, int offset, int buf_len);
+
+  // Flushes the in-memory data to the backing storage.
+  bool Flush(int index);
 
-  // Flush the in-memory data to the backing storage.
-  bool Flush(int index, int size, bool async);
+  // Updates the size of a given data stream.
+  void UpdateSize(int index, int old_size, int new_size);
 
   // Initializes the sparse control object. Returns a net error code.
   int InitSparseData();
@@ -195,7 +207,7 @@ class EntryImpl : public Entry, public base::RefCounted<EntryImpl> {
   CacheEntryBlock entry_;     // Key related information for this entry.
   CacheRankingsBlock node_;   // Rankings related information for this entry.
   BackendImpl* backend_;      // Back pointer to the cache.
-  scoped_array<char> user_buffers_[kNumStreams];  // Store user data.
+  scoped_ptr<UserBuffer> user_buffers_[kNumStreams];  // Stores user data.
   // Files to store external user data and key.
   scoped_refptr<File> files_[kNumStreams + 1];
   mutable std::string key_;           // Copy of the key.

net/disk_cache/file.h

@@ -59,10 +59,6 @@ class File : public base::RefCounted<File> {
   bool Write(const void* buffer, size_t buffer_len, size_t offset,
              FileIOCallback* callback, bool* completed);
 
-  // Performs asynchronous writes, but doesn't notify when done. Automatically
-  // deletes buffer when done.
-  bool PostWrite(const void* buffer, size_t buffer_len, size_t offset);
-
   // Sets the file's length. The file is truncated or extended with zeros to
   // the new length.
   bool SetLength(size_t length);
@@ -77,7 +73,7 @@ class File : public base::RefCounted<File> {
   // Performs the actual asynchronous write. If notify is set and there is no
   // callback, the call will be re-synchronized.
   bool AsyncWrite(const void* buffer, size_t buffer_len, size_t offset,
-                  bool notify, FileIOCallback* callback, bool* completed);
+                  FileIOCallback* callback, bool* completed);
 
  private:
   bool init_;

net/disk_cache/file_posix.cc

@@ -37,12 +37,11 @@ class BackgroundIO : public base::RefCountedThreadSafe<BackgroundIO> {
 
   // Read and Write are the operations that can be performed asynchronously.
   // The actual parameters for the operation are setup in the constructor of
-  // the object, with the exception of |delete_buffer|, that allows a write
-  // without a callback. Both methods should be called from a worker thread, by
-  // posting a task to the WorkerPool (they are RunnableMethods). When finished,
+  // the object. Both methods should be called from a worker thread, by posting
+  // a task to the WorkerPool (they are RunnableMethods). When finished,
   // controller->OnIOComplete() is called.
   void Read();
-  void Write(bool delete_buffer);
+  void Write();
 
   // This method signals the controller that this operation is finished, in the
   // original thread (presumably the IO-Thread). In practice, this is a
@@ -122,8 +121,7 @@ class InFlightIO {
   void PostRead(disk_cache::File* file, void* buf, size_t buf_len,
                 size_t offset, disk_cache::FileIOCallback* callback);
   void PostWrite(disk_cache::File* file, const void* buf, size_t buf_len,
-                 size_t offset, disk_cache::FileIOCallback* callback,
-                 bool delete_buffer);
+                 size_t offset, disk_cache::FileIOCallback* callback);
 
   // Blocks the current thread until all IO operations tracked by this object
   // complete.
@@ -167,12 +165,8 @@ void BackgroundIO::Cancel() {
 }
 
 // Runs on a worker thread.
-void BackgroundIO::Write(bool delete_buffer) {
+void BackgroundIO::Write() {
   bool rv = file_->Write(buf_, buf_len_, offset_);
-  if (delete_buffer) {
-    // TODO(rvargas): remove or update this code.
-    delete[] reinterpret_cast<const char*>(buf_);
-  }
 
   bytes_ = rv ? static_cast<int>(buf_len_) : -1;
   controller_->OnIOComplete(this);
@@ -203,8 +197,7 @@ void InFlightIO::PostRead(disk_cache::File *file, void* buf, size_t buf_len,
 
 void InFlightIO::PostWrite(disk_cache::File* file, const void* buf,
                            size_t buf_len, size_t offset,
-                           disk_cache::FileIOCallback* callback,
-                           bool delete_buffer) {
+                           disk_cache::FileIOCallback* callback) {
   scoped_refptr<BackgroundIO> operation =
       new BackgroundIO(file, buf, buf_len, offset, callback, this);
   io_list_.insert(operation.get());
@@ -214,8 +207,7 @@ void InFlightIO::PostWrite(disk_cache::File* file, const void* buf,
     callback_thread_ = MessageLoop::current();
 
   WorkerPool::PostTask(FROM_HERE,
-                       NewRunnableMethod(operation.get(), &BackgroundIO::Write,
-                                         delete_buffer),
+                       NewRunnableMethod(operation.get(), &BackgroundIO::Write),
                        true);
 }
 
@@ -342,22 +334,17 @@ bool File::Write(const void* buffer, size_t buffer_len, size_t offset,
     return Write(buffer, buffer_len, offset);
   }
 
-  return AsyncWrite(buffer, buffer_len, offset, true, callback, completed);
-}
-
-bool File::PostWrite(const void* buffer, size_t buffer_len, size_t offset) {
-  DCHECK(init_);
-  return AsyncWrite(buffer, buffer_len, offset, false, NULL, NULL);
+  return AsyncWrite(buffer, buffer_len, offset, callback, completed);
 }
 
 bool File::AsyncWrite(const void* buffer, size_t buffer_len, size_t offset,
-                      bool notify, FileIOCallback* callback, bool* completed) {
+                      FileIOCallback* callback, bool* completed) {
   DCHECK(init_);
   if (buffer_len > ULONG_MAX || offset > ULONG_MAX)
     return false;
 
   InFlightIO* io_operations = Singleton<InFlightIO>::get();
-  io_operations->PostWrite(this, buffer, buffer_len, offset, callback, !notify);
+  io_operations->PostWrite(this, buffer, buffer_len, offset, callback);
 
   if (completed)
     *completed = false;

net/disk_cache/file_win.cc

@@ -15,7 +15,7 @@ namespace {
 struct MyOverlapped {
   MyOverlapped(disk_cache::File* file, size_t offset,
                disk_cache::FileIOCallback* callback);
-  ~MyOverlapped();
+  ~MyOverlapped() {}
   OVERLAPPED* overlapped() {
     return &context_.overlapped;
   }
@@ -23,8 +23,6 @@ struct MyOverlapped {
   MessageLoopForIO::IOContext context_;
   scoped_refptr<disk_cache::File> file_;
   disk_cache::FileIOCallback* callback_;
-  const void* buffer_;
-  bool delete_buffer_;  // Delete the user buffer at completion.
 };
 
 COMPILE_ASSERT(!offsetof(MyOverlapped, context_), starts_with_overlapped);
@@ -60,15 +58,6 @@ MyOverlapped::MyOverlapped(disk_cache::File* file, size_t offset,
   callback_ = callback;
 }
 
-MyOverlapped::~MyOverlapped() {
-  if (delete_buffer_) {
-    DCHECK(!callback_);
-    // This whole thing could be updated to use IOBuffer, but PostWrite is not
-    // used at the moment. TODO(rvargas): remove or update this code.
-    delete[] reinterpret_cast<const char*>(buffer_);
-  }
-}
-
 }  // namespace
 
 namespace disk_cache {
@@ -207,29 +196,18 @@ bool File::Write(const void* buffer, size_t buffer_len, size_t offset,
     return Write(buffer, buffer_len, offset);
   }
 
-  return AsyncWrite(buffer, buffer_len, offset, true, callback, completed);
-}
-
-bool File::PostWrite(const void* buffer, size_t buffer_len, size_t offset) {
-  DCHECK(init_);
-  return AsyncWrite(buffer, buffer_len, offset, false, NULL, NULL);
+  return AsyncWrite(buffer, buffer_len, offset, callback, completed);
 }
 
 bool File::AsyncWrite(const void* buffer, size_t buffer_len, size_t offset,
-                      bool notify, FileIOCallback* callback, bool* completed) {
+                      FileIOCallback* callback, bool* completed) {
   DCHECK(init_);
+  DCHECK(callback);
+  DCHECK(completed);
   if (buffer_len > ULONG_MAX || offset > ULONG_MAX)
     return false;
 
   MyOverlapped* data = new MyOverlapped(this, offset, callback);
-  bool dummy_completed;
-  if (!callback) {
-    DCHECK(!notify);
-    data->delete_buffer_ = true;
-    data->buffer_ = buffer;
-    completed = &dummy_completed;
-  }
-
   DWORD size = static_cast<DWORD>(buffer_len);
 
   DWORD actual;