[BlobAsync] Disk support for blob storage

storage/browser/blob/blob_memory_controller.h

Index: storage/browser/blob/blob_memory_controller.h
diff --git a/storage/browser/blob/blob_memory_controller.h b/storage/browser/blob/blob_memory_controller.h
new file mode 100644
index 0000000000000000000000000000000000000000..6a38bbce1bac66395d8734b807a08a2c22e0585b
--- /dev/null
+++ b/storage/browser/blob/blob_memory_controller.h
@@ -0,0 +1,223 @@
+// Copyright 2016 The Chromium Authors. All rihts reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef STORAGE_BROWSER_BLOB_BLOB_MEMORY_CONTROLLER_H_
+#define STORAGE_BROWSER_BLOB_BLOB_MEMORY_CONTROLLER_H_
+
+#include <stdint.h>
+
+#include <algorithm>
+#include <list>
+#include <map>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/containers/mru_cache.h"
+#include "base/files/file.h"
+#include "base/files/file_path.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/optional.h"
+#include "base/time/time.h"
+#include "storage/browser/storage_browser_export.h"
+#include "storage/common/blob_storage/blob_storage_constants.h"
+#include "storage/common/data_element.h"
+
+namespace base {
+class SequencedWorkerPool;
+}
+
+namespace storage {
+class ShareableBlobDataItem;
+class ShareableFileReference;
+
+// There should be no memory sizes of 0.
+// This class is responsible for
+// * our current memory usage for blobs, both on disk and in memory,
+// * when we can add a blob w/ a given size,
+// * creating temporary files,
+// * paging blobs to disk.
+class STORAGE_EXPORT BlobMemoryController {
+ public:
+  enum class MemoryStrategyResult {
+    // We don't have enough memory for this blob.
+    TOO_LARGE,
+    // There isn't any memory that needs transporting.
+    NONE_NEEDED,

[Marijn Kruisselbrink, 2016/07/12 21:33:06]

Is there any meaningful difference between NONE_NEEDED and SHORTCUT? It seems
both of them pretty much mean you don't need to transport any memory anymore;
and you seem to do the exact same thing for both values.

[dmurph, 2016/07/14 01:04:31]

Good idea, I'll remove shortcut.

+    // Transportation strategies.
+    SHORTCUT,
+    IPC,
+    SHARED_MEMORY,
+    FILE
+  };
+  typedef std::list<std::pair<size_t, base::Callback<void(bool)>>>

[Marijn Kruisselbrink, 2016/07/12 21:33:06]

using instead of typedef (also below)

[dmurph, 2016/07/14 01:04:31]

Done.

+      PendingBlobConstructionList;
+  typedef PendingBlobConstructionList::iterator PendingConstructionEntry;

[michaeln, 2016/07/14 01:44:43]

This typedef is odd?

[dmurph, 2016/07/15 02:45:27]

I need that entry defined as  I return it below.

+
+  struct FileCreationInfo {
+    FileCreationInfo();
+    ~FileCreationInfo();
+    FileCreationInfo(FileCreationInfo&& other);
+    FileCreationInfo& operator=(FileCreationInfo&&);
+
+    base::File::Error error = base::File::FILE_ERROR_FAILED;
+    scoped_refptr<ShareableFileReference> file_reference;
+    base::File file;
+    base::Time last_modified;
+  };
+
+  BlobMemoryController();
+  virtual ~BlobMemoryController();
+
+  void EnableDisk(const base::FilePath& storage_directory,
+                  scoped_refptr<base::SequencedWorkerPool> file_worker_pool);

[michaeln, 2016/07/14 01:44:43]

+1 putting these tasks on the worker pool instead of a well known thread.

Instead of passing the SequencedWorkerPool itself, all thats really needed is a
TaskRunner backed by the pool. This makes testing easier too.

It might be a good idea to have this class use a SequentedTaskRunner to ensure
that the blob system does not tie up all of the threads of the pool at any given
time.

[dmurph, 2016/07/15 02:45:27]

Done.

+
+  // Disables the disk.
+  void DisableDisk();
+
+  bool disk_enabled() const { return disk_used_; }
+
+  // Returns if the data in the descriptions is good data (not a bad IPC). If
+  // SHORTCUT is returned, then the blob can be immediately constructed, and
+  // the a following call to MaybeFitInMemoryNow will always return true.

[Marijn Kruisselbrink, 2016/07/12 21:33:06]

the a?

[dmurph, 2016/07/14 01:04:31]

Done.

+  bool DecideBlobTransportationMemoryStrategy(
+      const std::vector<DataElement>& descriptions,
+      uint64_t* total_bytes,
+      MemoryStrategyResult* result) const;
+
+  // Creates a temporary file that we store in our blob temporary directory. The
+  // lifetime of the file is tied to the file_reference.
+  void CreateTemporaryFileForRenderer(

[michaeln, 2016/07/14 01:44:43]

naming nit: "ForRenderer" isn't really needed

[dmurph, 2016/07/15 02:45:27]

Done.

+      uint64_t size_bytes,
+      const base::Callback<void(FileCreationInfo)>& file_callback);
+
+  void FreeMemory(size_t memory_size_bytes);
+
+  enum class MemoryRequestResponse {

[Marijn Kruisselbrink, 2016/07/12 21:33:06]

https://google.github.io/styleguide/cppguide.html#Declaration_Order says that
generally all type definitions in a class should come before all methods.

Actually, are you even using this enum anywhere?

[dmurph, 2016/07/14 01:04:31]

Removed!

+    SPACE_AVAILABLE,
+    SPACE_UNAVAILABLE,
+    PENDING_DISK
+  };
+
+  bool CanFitInSystem(uint64_t size) const;
+
+  // We assume the user called CanFitInSystem before this and that the given
+  // memory size can fit in our system.
+  // Notifies the caller when a blob of the given size can be stored in our blob
+  // system. If the returned entry is not present, that means the blob can fit
+  // right away and the callback won't be called.
+  // The argument on can_request is true if we can create the blob, and
+  // false if there was an error and the new blob needs to be broken.
+  base::Optional<PendingConstructionEntry> NotifyWhenMemoryCanPopulated(
+      size_t memory_size_bytes,
+      const base::Callback<void(bool)>& can_request);
+
+  void RemovePendingConstructionEntry(const PendingConstructionEntry& entry);
+
+  // This updates the given item in our LRU lookup table.
+  void UpdateBlobItemInRecents(ShareableBlobDataItem* item);
+
+  // This removes the given item from our LRU lookup table.
+  void RemoveBlobItemInRecents(const ShareableBlobDataItem& item);
+
+  size_t memory_usage() const {
+    return blob_memory_used_ + in_flight_memory_used_;
+  }
+  uint64_t disk_usage() const { return disk_used_; }
+
+  size_t max_ipc_memory_size() const { return max_ipc_memory_size_; }
+  size_t max_shared_memory_size() const { return max_shared_memory_size_; }
+  size_t max_blob_in_memory_size() const { return max_blob_in_memory_size_; }
+  size_t max_blob_memory_space() const { return max_blob_memory_space_; }
+  uint64_t max_blob_disk_space() const { return max_blob_disk_space_; }
+  size_t in_flight_space() const { return in_flight_space_; }
+  uint64_t min_page_file_size() const { return min_page_file_size_; }
+  uint64_t max_file_size() const { return max_file_size_; }
+
+  void SetMemoryConstantsForTesting(size_t max_ipc_memory_size,
+                                    size_t max_shared_memory_size,
+                                    size_t max_blob_in_memory_size,
+                                    size_t max_blob_memory_space,

[michaeln, 2016/07/14 01:44:43]

what's the difference between max_blob_in_memory_size and max_blob_memory_space?

[dmurph, 2016/07/15 02:45:27]

Removed. One was the addition of max blob size and in flight size.

+                                    uint64_t max_blob_disk_space,
+                                    size_t in_flight_space,
+                                    uint64_t min_page_file_size,
+                                    uint64_t max_file_size) {

[michaeln, 2016/07/14 01:44:43]

is there a reason the "min" value has "page" in the name and the max value does
not?

[dmurph, 2016/07/15 02:45:27]

The min size is used for paging, and the max size is used for files for the
renderer.

+    max_ipc_memory_size_ = max_ipc_memory_size;
+    max_shared_memory_size_ = max_shared_memory_size;
+    max_blob_in_memory_size_ = max_blob_in_memory_size;
+    max_blob_memory_space_ = max_blob_memory_space;
+    max_blob_disk_space_ = max_blob_disk_space;
+    in_flight_space_ = in_flight_space;
+    min_page_file_size_ = min_page_file_size;
+    max_file_size_ = max_file_size;
+  }
+
+ private:
+  using RecentItemsCache = base::MRUCache<uint64_t, ShareableBlobDataItem*>;
+
+  // We add a delete callback to the file reference to decrement our disk usage.
+  void OnCreateFile(uint64_t file_size,
+                    const base::Callback<void(FileCreationInfo)>& done,
+                    FileCreationInfo result);
+
+  void MaybeScheduleWaitingBlobs();
+
+  // Returns the size we'll be freeing. We appropriately increment/decrement
+  // the disk, memory, and in flight counts.
+  size_t ScheduleBlobPaging();
+  void SchedulePagingUntilWeCanFit(size_t total_memory_needed);
+
+  // Called when we've completed paging a list of items
+  void OnPagingComplete(
+      std::unique_ptr<std::vector<scoped_refptr<ShareableBlobDataItem>>> items,
+      size_t total_items_size,
+      FileCreationInfo result);
+
+  void RecordTracingCounters();
+
+  bool ShouldSchedulePagingForSize(size_t size) const;
+  size_t GetAvailableMemoryForBlobs() const;
+  uint64_t GetAvailableDiskSpaceForBlobs() const;
+
+  // We decrement the disk space used.
+  void OnBlobFileDelete(uint64_t size, const base::FilePath& path);
+
+  // Memory bookkeeping.
+  size_t blob_memory_used_ = 0;
+  size_t in_flight_memory_used_ = 0;
+  uint64_t disk_used_ = 0;
+
+  scoped_refptr<base::SequencedWorkerPool> file_worker_pool_;
+
+  size_t pending_pagings_ = 0;
+  PendingBlobConstructionList blobs_waiting_for_paging_;
+  size_t blobs_waiting_for_paging_size_ = 0;
+
+  // Constants.

[michaeln, 2016/07/14 01:44:43]

A struct for these constant/config/settings would be nice.

[dmurph, 2016/07/15 02:45:27]

put a TODO.

+  size_t max_ipc_memory_size_ = kBlobStorageIPCThresholdBytes;
+  size_t max_shared_memory_size_ = kBlobStorageMaxSharedMemoryBytes;
+  size_t max_blob_in_memory_size_ = kBlobStorageMaxBlobMemorySize;
+  size_t max_blob_memory_space_ = kBlobStorageMaxMemoryUsage;
+  uint64_t max_blob_disk_space_ = kBlobStorageMaxDiskSpace;
+  size_t in_flight_space_ = kBlobStorageInFlightMemory;
+  uint64_t min_page_file_size_ = kBlobStorageMinFileSizeBytes;
+  uint64_t max_file_size_ = kBlobStorageMaxFileSizeBytes;
+
+  bool enable_disk_ = false;
+  base::FilePath blob_storage_dir_;
+  uint64_t current_file_num_ = 0;
+
+  // Lifetime of the ShareableBlobDataItem objects is handled externally in the
+  // BlobStorageContext class.
+  RecentItemsCache recent_item_cache_;

[michaeln, 2016/07/14 01:44:43]

The using alias for RecentItemsCache isn't needed, the alias is only used on
this line. It'd be a little clearer to use the base::MRUCache<> inline, the
comment referring to the rawptr would make more sense.

[dmurph, 2016/07/15 02:45:27]

Done.

+
+  base::WeakPtrFactory<BlobMemoryController> ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(BlobMemoryController);
+};
+}  // namespace storage
+#endif  // STORAGE_BROWSER_BLOB_BLOB_MEMORY_CONTROLLER_H_