[BlobStorage] BlobMemoryController & tests

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..99cb0c4eedea21a8997563c2401afa6e82482b08
--- /dev/null
+++ b/storage/browser/blob/blob_memory_controller.h
@@ -0,0 +1,237 @@
+// Copyright 2016 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.
+
+#ifndef STORAGE_BROWSER_BLOB_BLOB_MEMORY_CONTROLLER_H_
+#define STORAGE_BROWSER_BLOB_BLOB_MEMORY_CONTROLLER_H_
+
+#include <stdint.h>
+
+#include <list>
+#include <map>
+#include <memory>
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#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"
+
+namespace base {
+class TaskRunner;
+}
+
+namespace storage {
+class DataElement;
+class ShareableBlobDataItem;
+class ShareableFileReference;
+
+// This class's main responsability is deciding how blob data gets stored.
+// This encompasses:
+// * Keeping track of memory & file quota,
+// * How to transport the blob data from the renderer (DetermineStrategy),
+// * Allocating memory & file quota (ReserveMemoryQuota, ReserveFileQuota)
+// * Freeing quota (MaybeFreeQuotaForItems)
+// * Paging memory quota to disk when we're nearing our memory limit, and
+// * Maintaining an LRU of memory items to choose candidates to page to disk
+//   (UpdateBlobItemInRecents, RemoveBlobItemInRecents).

[michaeln, 2016/10/04 00:11:55]

the methods names in the comment are stale now

[dmurph, 2016/10/06 00:45:39]

Done.

+// This class can only be interacted with on the IO thread.
+class STORAGE_EXPORT BlobMemoryController {
+ public:
+  enum class Strategy {
+    // We don't have enough memory for this blob.
+    TOO_LARGE,
+    // There isn't any memory that needs transporting.
+    NONE_NEEDED,
+    // Transportation strategies.
+    IPC,
+    SHARED_MEMORY,
+    FILE
+  };
+
+  struct FileCreationInfo {
+    FileCreationInfo();
+    ~FileCreationInfo();
+    FileCreationInfo(FileCreationInfo&& other);
+    FileCreationInfo& operator=(FileCreationInfo&&);
+
+    base::File::Error error = base::File::FILE_ERROR_FAILED;
+    base::File file;
+    base::FilePath path;
+    scoped_refptr<ShareableFileReference> file_reference;
+    base::Time last_modified;
+  };
+
+  // The bool argument is true if we successfully received memory quota.
+  using MemoryQuotaRequestCallback = base::Callback<void(bool)>;
+  // The bool argument is true if we successfully received file quota, and the
+  // vector argument provides the file info.
+  using FileQuotaRequestCallback =
+      base::Callback<void(bool, std::vector<FileCreationInfo>)>;
+
+  // IO thread only.

[michaeln, 2016/10/04 00:11:55]

since the outerclass says io only, maybe this comment isn't needed here

[dmurph, 2016/10/06 00:45:39]

Done.

+  class QuotaAllocationTask {

[michaeln, 2016/10/04 00:11:55]

style nit: declaration order, i think the struct and class should be adjacent to
one another and the using alias's coming after

[dmurph, 2016/10/06 00:45:39]

Done.

+   public:
+    // Operation is cancelled and the callback will NOT be called. This object
+    // will be destroyed by this call.
+    virtual void Cancel() = 0;
+
+   protected:
+    virtual ~QuotaAllocationTask();
+  };
+
+  // We enable file paging if |file_runner| isn't a nullptr.
+  BlobMemoryController(const base::FilePath& storage_directory,
+                       scoped_refptr<base::TaskRunner> file_runner);
+  virtual ~BlobMemoryController();
+
+  // Disables the disk. This cancels all pending file creations and paging
+  // operations.
+  void DisableFilePaging();
+
+  bool file_paging_enabled() const { return file_paging_enabled_; }
+
+  // Returns the strategy the transportation layer should use to transport the
+  // given memory. |preemptive_transported_bytes| are the number of transport
+  // bytes that are already populated for us, so we don't haved to request them
+  // from the renderer.
+  Strategy DetermineStrategy(size_t preemptive_transported_bytes,
+                             uint64_t total_transportation_bytes) const;
+
+  // Checks to see if we can reserve quota (disk or memory) for the given size.
+  bool CanReserveQuota(uint64_t size) const;
+
+  // Reserves quota for the given |unreserved_memory_items|. The items must be
+  // bytes items in QUOTA_NEEDED state, which we change to QUOTA_REQUESTED.
+  // After we reserve memory quota we change their state to QUOTA_GRANTED and
+  // call |done_callback|.  This can happen synchronously.
+  // Returns a task handle if the request is asynchronous for cancellation.
+  // NOTE: We don't inspect quota limits and assume the user checked
+  //       CanReserveQuota before calling this.
+  base::WeakPtr<QuotaAllocationTask> ReserveMemoryQuota(
+      std::vector<ShareableBlobDataItem*> unreserved_memory_items,

[michaeln, 2016/10/04 00:11:55]

would it work to make the vector a const ref? its funky that the unittests
constructs a vector a scoped_refs and seperated has to muck with a vector of
ptrs

[dmurph, 2016/10/06 00:45:39]

So the file quota one always needs to grab a copy of the items, so if we want
them to be the same I'd like to keep this as a vector by-value. I then std::move
it if I need to, but the bad side of this matching is that I'm increasing the
refcount of the items when I don't necessarily need to keep them (whereas this
is always the case in the file quota one).

+      const MemoryQuotaRequestCallback& done_callback);
+
+  // Reserves quota for the given |unreserved_file_items|. The items must be
+  // temporary file items (BlobDataBuilder::IsTemporaryFileItem returns true) in
+  // QUOTA_NEEDED state, which we change to QUOTA_REQUESTED. After we reserve
+  // file quota we change their state to QUOTA_GRANTED and call
+  // |done_callback|.
+  // Returns a task handle for cancellation.
+  // NOTE: We don't inspect quota limits and assume the user checked
+  //       CanReserveQuota before calling this.
+  base::WeakPtr<QuotaAllocationTask> ReserveFileQuota(
+      std::vector<scoped_refptr<ShareableBlobDataItem>> unreserved_file_items,

[michaeln, 2016/10/04 00:11:55]

i'd suggest a const ref vector<> here too for symmetry with the other method,
they're essentially the same thing, an arbitrary difference calls unnecessary
attention. making them look boringly alike might be good

[dmurph, 2016/10/06 00:45:39]

So we want to save this vector and send it to another thread, so I like sending
it by value and using std::move.

+      const FileQuotaRequestCallback& done_callback);
+
+  // This frees quota for items that are not referenced by any blobs.
+  void MaybeFreeMemoryQuotaForItems(
+      const std::vector<scoped_refptr<ShareableBlobDataItem>>& items);
+  // Forcibly frees memory quota for given item. Item must have granted quota.
+  void ForceFreeMemoryQuotaForItem(
+      const scoped_refptr<ShareableBlobDataItem>& item);
+
+  // Called when initially populated or upon later access.
+  void NotifyMemoryItemUsed(ShareableBlobDataItem* item);
+
+  size_t memory_usage() const {
+    return blob_memory_used_ + in_flight_memory_used_;
+  }
+  uint64_t disk_usage() const { return disk_used_; }
+
+  const BlobStorageLimits& limits() const { return limits_; }
+  void SetLimitsForTesting(BlobStorageLimits limits) { limits_ = limits; }

[michaeln, 2016/10/04 00:11:55]

nit: unix_hacker_naming()

[dmurph, 2016/10/06 00:45:39]

Done.

+
+ private:
+  template <typename T>
+  class BaseQuotaAllocationTask;
+  class FileQuotaAllocationTask;
+  class MemoryQuotaAllocationTask;
+
+  using PendingMemoryQuotaTaskList =
+      std::list<std::unique_ptr<MemoryQuotaAllocationTask>>;
+  using PendingFileQuotaTaskList =
+      std::list<std::unique_ptr<FileQuotaAllocationTask>>;
+
+  // Removes the given blob item from our LRU list for paging.
+  void RemoveItemInRecents(const ShareableBlobDataItem& item);
+
+  void MaybeGrantPendingQuotaRequests();
+
+  size_t CollectItemsForEviction(
+      std::vector<scoped_refptr<ShareableBlobDataItem>>* output);
+
+  // Schedule paging until our memory usage is below our memory limit.
+  void MaybeScheduleEvictionUntilSystemHealthy();
+
+  // Called when we've completed evicting a list of items to disk. This is where
+  // we swap the bytes items for file items, and and update our bookkeeping.
+  void OnEvictionComplete(
+      scoped_refptr<ShareableFileReference> file_reference,
+      std::vector<scoped_refptr<ShareableBlobDataItem>> items,
+      size_t total_items_size,
+      FileCreationInfo result);
+
+  size_t GetAvailableMemoryForBlobs() const;
+  uint64_t GetAvailableFileSpaceForBlobs() const;
+
+  // This is registered as a callback for file deletions on the file reference
+  // of our paging files. We decrement the disk space used.
+  void OnBlobFileDelete(uint64_t size, const base::FilePath& path);
+
+  base::FilePath GenerateNextPageFileName();
+
+  // This records diagnostic counters of our memory quotas. Called when usage
+  // changes.
+  void RecordTracingCounters() const;
+
+  BlobStorageLimits limits_;
+
+  // Memory bookkeeping. These numbers are all disjoint.
+  // This is the amount of memory we're using for blobs in RAM.
+  size_t blob_memory_used_ = 0;
+  // This is memory we're temporarily using while we try to write blob items to
+  // disk.
+  size_t in_flight_memory_used_ = 0;
+  // This is the amount of memory we're using on disk.
+  uint64_t disk_used_ = 0;
+
+  // State for GenerateNextPageFileName.
+  uint64_t current_file_num_ = 0;
+
+  size_t pending_memory_quota_total_size_ = 0;
+  PendingMemoryQuotaTaskList pending_memory_quota_tasks_;
+  PendingFileQuotaTaskList pending_file_quota_tasks_;
+
+  size_t pending_evictions_ = 0;
+
+  bool file_paging_enabled_ = false;
+  base::FilePath blob_storage_dir_;
+  scoped_refptr<base::TaskRunner> file_runner_;
+
+  // Lifetime of the ShareableBlobDataItem objects is handled externally in the
+  // BlobStorageContext class.
+  base::MRUCache<uint64_t, ShareableBlobDataItem*> recent_item_cache_;
+  size_t recent_item_cache_bytes_ = 0;
+  // We need to keep track of items currently being paged to disk so that if
+  // another blob successfully grabs a ref, we can prevent it from adding the
+  // item to the recent_item_cache_ above.

[michaeln, 2016/10/04 00:11:55]

would it make sense to represent this as a distinct ShareableBlobDataItem state
value instead of in this seperate collection? nevermind, just looked at those
state values, probably not a good idea to mix in type transitions to that set of
values.

[dmurph, 2016/10/06 00:45:39]

Acknowledged.

+  std::unordered_set<uint64_t> items_saving_to_disk_;
+
+  base::WeakPtrFactory<BlobMemoryController> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(BlobMemoryController);
+};
+}  // namespace storage
+#endif  // STORAGE_BROWSER_BLOB_BLOB_MEMORY_CONTROLLER_H_