[Courgette] PagedArray: Add Iterators and Parametrize Page Size as int Template.

courgette/third_party/bsdiff/paged_array.h

Index: courgette/third_party/bsdiff/paged_array.h
diff --git a/courgette/third_party/bsdiff/paged_array.h b/courgette/third_party/bsdiff/paged_array.h
index 27c3c0b37db7fbfce7038aed7728f7a333477526..4de69766bfb821c2dd2aee731b9fdfcc22d18eb5 100644
--- a/courgette/third_party/bsdiff/paged_array.h
+++ b/courgette/third_party/bsdiff/paged_array.h
@@ -13,25 +13,141 @@
 
 #include <stddef.h>
 
+#include <cstddef>

[etiennep, 2016/05/26 14:03:29]

Redundant with stddef.h

[huangs, 2016/05/26 17:54:27]

Done.

+#include <iterator>
+#include <type_traits>
+
+#include "base/logging.h"
 #include "base/macros.h"
 #include "base/process/memory.h"
 
 namespace courgette {
 
+// Page size of 2^18 * sizeof(T) is 1MB for T = int32_t.
+constexpr int kPagedArrayDefaultPageLogSize = 18;
+
+template <typename T, int LOG_PAGE_SIZE = kPagedArrayDefaultPageLogSize>
+class PagedArray;
+
+template <typename ContainerType, typename T>
+class PagedArray_iterator {
+ public:
+  using ThisType = PagedArray_iterator<ContainerType, T>;
+  using difference_type = ptrdiff_t;
+  using value_type = typename std::remove_const<T>::type;
+  using reference = T&;
+  using pointer = T*;
+  using iterator_category = std::random_access_iterator_tag;
+
+  PagedArray_iterator() : array_(nullptr), index_(0U) { }
+  PagedArray_iterator(ContainerType* array, size_t index)
+      : array_(array), index_(index) { }
+
+  template <typename ContainerType2, typename T2>
+  PagedArray_iterator(const PagedArray_iterator<ContainerType2, T2>& it)
+      : array_(it.array_), index_(it.index_) { }
+
+  PagedArray_iterator(std::nullptr_t)
+      : array_(nullptr), index_(0) { }
+
+  virtual ~PagedArray_iterator() = default;

[etiennep, 2016/05/26 14:03:30]

I don't think virtual is necessary anymore.

[huangs, 2016/05/26 17:54:27]

Done.

+
+  reference operator*() const { return (*array_)[index_]; }
+  reference operator[](size_t idx) const { return (*array_)[index_ + idx]; }
+  pointer operator->() const { return &(*array_)[index_]; }
+
+  ThisType& operator=(std::nullptr_t) {
+    array_ = nullptr;
+    index_ = 0;
+    return *this;
+  }
+
+  ThisType& operator++() {
+    ++index_;
+    return *this;
+  }
+  ThisType& operator--() {
+    --index_;
+    return *this;
+  }
+
+  ThisType operator++(int) { return ThisType(array_, index_++); }
+  ThisType operator--(int) { return ThisType(array_, index_--); }
+
+  ThisType& operator+=(difference_type delta) {
+    index_ += delta;
+    return *this;
+  }
+  ThisType& operator-=(difference_type delta) {
+    index_ -= delta;
+    return *this;
+  }
+
+  ThisType operator+(difference_type offset) const {
+    return ThisType(array_, index_ + offset);
+  }
+  ThisType operator-(difference_type offset) const {
+    return ThisType(array_, index_ - offset);
+  }
+
+  template <typename ContainerType2, typename T2>
+  bool operator==(const PagedArray_iterator<ContainerType2, T2>& it) const {
+    return index_ == it.index_ && array_ == it.array_;
+  }
+  bool operator==(std::nullptr_t) const {
+    return index_ == 0 && array_ == nullptr;
+  }
+  template <typename ContainerType2, typename T2>
+  bool operator!=(const PagedArray_iterator<ContainerType2, T2>& it) const {
+    return !(*this == it);
+  }
+
+  template <typename ContainerType2, typename T2>
+  bool operator<(const PagedArray_iterator<ContainerType2, T2>& it) const {
+    // Not bothering to check array_, for performance.
+    return index_ < it.index_;
+  }
+  template <typename ContainerType2, typename T2>
+  bool operator<=(const PagedArray_iterator<ContainerType2, T2>& it) const {
+    // Not bothering to check array_, for performance.
+    return index_ <= it.index_;
+  }
+
+  template <typename ContainerType2, typename T2>
+  difference_type operator-(const PagedArray_iterator<ContainerType2, T2>& it) const {

[etiennep, 2016/05/26 14:03:29]

Line too long?

[huangs, 2016/05/26 17:54:27]

Done.

+    return index_ - it.index_;
+  }
+
+ private:
+  template <typename, typename>
+  friend class PagedArray_iterator;
+
+  ContainerType* array_;
+  size_t index_;
+};
+
 // PagedArray implements an array stored using many fixed-size pages.
-template <typename T>
+template <typename T, int LOG_PAGE_SIZE>
 class PagedArray {
   enum {
-    // Page size in elements.  Page size of 2^18 * sizeof(T) is 1MB for T = int.
-    kLogPageSize = 18,
-    kPageSize = 1 << kLogPageSize
+    // Page size in elements.
+    kLogPageSize = LOG_PAGE_SIZE,
+    kPageSize = 1 << LOG_PAGE_SIZE
   };
 
  public:
-  PagedArray() : pages_(NULL), page_count_(0) {}
+  using ThisType = PagedArray<T, LOG_PAGE_SIZE>;
+  using const_iterator = PagedArray_iterator<const ThisType, const T>;
+  using iterator = PagedArray_iterator<ThisType, T>;
 
+  PagedArray() { }

[etiennep, 2016/05/26 14:03:29]

= default

[huangs, 2016/05/26 17:54:27]

Done.

   ~PagedArray() { clear(); }
 
+  iterator begin() { return iterator(this, 0); }
+  iterator end() { return iterator(this, size_); }
+  const_iterator begin() const { return const_iterator(this, 0); }
+  const_iterator end() const { return const_iterator(this, size_); }
+
   T& operator[](size_t i) {
     size_t page = i >> kLogPageSize;
     size_t offset = i & (kPageSize - 1);
@@ -40,11 +156,20 @@ class PagedArray {
     return pages_[page][offset];
   }
 
+  const T& operator[](size_t i) const {

[etiennep, 2016/05/26 14:03:29]

Optionally, implement this one using the other one:
return const_cast<PagedArray&>(*this)[i];

[huangs, 2016/05/26 17:54:27]

I'd rather avoid unneeded use of const_cast<>.  Adding a common inline helper
routine that's used by both.

+    size_t page = i >> kLogPageSize;
+    size_t offset = i & (kPageSize - 1);
+    // It is tempting to add a DCHECK(page < page_count_), but that makes
+    // bsdiff_create run 2x slower (even when compiled optimized.)
+    return pages_[page][offset];
+  }
+
   // Allocates storage for |size| elements. Returns true on success and false if
   // allocation fails.
   bool Allocate(size_t size) {
     clear();
-    size_t pages_needed = (size + kPageSize - 1) >> kLogPageSize;
+    size_ = size;
+    size_t pages_needed = (size_ + kPageSize - 1) >> kLogPageSize;
     if (!base::UncheckedMalloc(sizeof(T*) * pages_needed,
                                reinterpret_cast<void**>(&pages_))) {
       return false;
@@ -64,22 +189,24 @@ class PagedArray {
 
   // Releases all storage.  May be called more than once.
   void clear() {
-    if (pages_ != NULL) {
+    if (pages_ != nullptr) {
       while (page_count_ != 0) {
         --page_count_;
         free(pages_[page_count_]);
       }
       free(pages_);
-      pages_ = NULL;
+      pages_ = nullptr;
     }
   }
 
  private:
-  T** pages_;
-  size_t page_count_;
+  T** pages_ = nullptr;
+  size_t size_ = 0U;
+  size_t page_count_ = 0U;

[etiennep, 2016/05/26 14:03:29]

If we have size_, we don't really need page_count_, as it can be easily computed
(and only used when calling clear)

[huangs, 2016/05/26 17:54:27]

True.  However, note that page_count_ is used in the Allocate()'s "ramp-up"
loop, and on failure is used to "ramp-down". Changing how this works is slightly
risky.  On principle, the change should not be bundled into this CL.

 
   DISALLOW_COPY_AND_ASSIGN(PagedArray);
 };
 
 }  // namespace courgette
+
 #endif  // COURGETTE_THIRD_PARTY_BSDIFF_PAGED_ARRAY_H_