Followup code refactor on plain table

Summary:
Fixed most comments in https://reviews.facebook.net/D15429.
Still have some remaining comments left.

Test Plan: make all check

Reviewers: sdong, haobo

Reviewed By: haobo

CC: leveldb

Differential Revision: https://reviews.facebook.net/D15885

table/plain_table_reader.h

@@ -69,7 +69,8 @@ class PlainTableReader: public TableReader {
     return table_properties_;
   }
 
-  PlainTableReader(const EnvOptions& storage_options,
+  PlainTableReader(const Options& options, unique_ptr<RandomAccessFile>&& file,
+                   const EnvOptions& storage_options,
                    const InternalKeyComparator& internal_comparator,
                    uint64_t file_size, int bloom_num_bits,
                    double hash_table_ratio, size_t index_sparseness,
@@ -85,9 +86,9 @@ class PlainTableReader: public TableReader {
   // PopulateIndex() builds index of keys. It must be called before any query
   // to the table.
   //
-  // hash_table_ contains buckets size of hash_table_size_, each is a 32-bit
-  // integer. The lower 31 bits contain an offset value (explained below) and
-  // the first bit of the integer indicates type of the offset.
+  // index_ contains buckets size of index_size_, each is a
+  // 32-bit integer. The lower 31 bits contain an offset value (explained below)
+  // and the first bit of the integer indicates type of the offset.
   //
   // +--------------+------------------------------------------------------+
   // | Flag (1 bit) | Offset to binary search buffer or file (31 bits)     +
@@ -122,23 +123,29 @@ class PlainTableReader: public TableReader {
   // <end>
   Status PopulateIndex();
 
-  Options options_;
-  unique_ptr<RandomAccessFile> file_;
-
  private:
   struct IndexRecord;
   class IndexRecordList;
 
-  uint32_t* hash_table_ = nullptr;
-  int hash_table_size_ = 0;
-  char* sub_index_ = nullptr;
+  // Plain table maintains an index and a sub index.
+  // index is implemented by a hash table.
+  // subindex is a big of memory array.
+  // For more details about the in-memory index, please refer to:
+  // https://github.com/facebook/rocksdb/wiki/PlainTable-Format
+  // #wiki-in-memory-index-format
+  std::unique_ptr<uint32_t[]> index_;
+  int index_size_ = 0;
+  std::unique_ptr<char[]> sub_index_;
 
+  Options options_;
   const EnvOptions& soptions_;
+  unique_ptr<RandomAccessFile> file_;
+
   const InternalKeyComparator internal_comparator_;
+  // represents plain table's current status.
   Status status_;
 
   Slice file_data_;
-  uint32_t version_;
   uint32_t file_size_;
 
   const double kHashTableRatio;
@@ -147,9 +154,12 @@ class PlainTableReader: public TableReader {
   // every N keys, where the "N" is determined by
   // kIndexIntervalForSamePrefixKeys
   const size_t kIndexIntervalForSamePrefixKeys = 16;
-  DynamicBloom* bloom_ = nullptr;
+  // Bloom filter is used to rule out non-existent key
+  unique_ptr<DynamicBloom> bloom_;
 
   std::shared_ptr<const TableProperties> table_properties_;
+  // data_start_offset_ and data_end_offset_ defines the range of the
+  // sst file that stores data.
   const uint32_t data_start_offset_ = 0;
   const uint32_t data_end_offset_;
   const size_t user_key_len_;
@@ -176,42 +186,43 @@ class PlainTableReader: public TableReader {
   // If bloom_ is not null, all the keys' full-key hash will be added to the
   // bloom filter.
   Status PopulateIndexRecordList(IndexRecordList* record_list,
-                                 int* num_prefixes, DynamicBloom* bloom_) const;
+                                 int* num_prefixes) const;
 
   // Internal helper function to allocate memory for indexes and bloom filters
   void AllocateIndexAndBloom(int num_prefixes);
 
   // Internal helper function to bucket index record list to hash buckets.
-  // hash_to_offsets is sized of of hash_table_size_, each contains a linked
-  // list
+  // bucket_header is a vector of size hash_table_size_, with each entry
+  // containing a linklist of IndexRecord hashed to the same bucket, in reverse
+  // order.
   // of offsets for the hash, in reversed order.
-  // bucket_count is sized of hash_table_size_. The value is how many index
-  // records are there in hash_to_offsets for the same bucket.
-  size_t BucketizeIndexesAndFillBloom(
-      IndexRecordList& record_list, int num_prefixes,
-      std::vector<IndexRecord*>* hash_to_offsets,
-      std::vector<uint32_t>* bucket_count);
+  // bucket_count is sized of index_size_. The value is how many index
+  // records are there in bucket_headers for the same bucket.
+  size_t BucketizeIndexesAndFillBloom(IndexRecordList* record_list,
+                                      std::vector<IndexRecord*>* bucket_headers,
+                                      std::vector<uint32_t>* bucket_count);
 
   // Internal helper class to fill the indexes and bloom filters to internal
-  // data structures. hash_to_offsets and bucket_count are bucketized indexes
+  // data structures. bucket_headers and bucket_count are bucketized indexes
   // and counts generated by BucketizeIndexesAndFillBloom().
   void FillIndexes(size_t sub_index_size_needed,
-                   const std::vector<IndexRecord*>& hash_to_offsets,
+                   const std::vector<IndexRecord*>& bucket_headers,
                    const std::vector<uint32_t>& bucket_count);
 
+  // Read a plain table key from the position `start`. The read content
+  // will be written to `key` and the size of read bytes will be populated
+  // in `bytes_read`.
   Status ReadKey(const char* row_ptr, ParsedInternalKey* key,
                  size_t* bytes_read) const;
-  // Read the key and value at offset to key and value.
-  // tmp_slice is a tmp slice.
-  // return next_offset as the offset for the next key.
-  Status Next(uint32_t offset, ParsedInternalKey* key, Slice* value,
-              uint32_t* next_offset) const;
+  // Read the key and value at `offset` to parameters `key` and `value`.
+  // On success, `offset` will be updated as the offset for the next key.
+  Status Next(uint32_t* offset, ParsedInternalKey* key, Slice* value) const;
   // Get file offset for key target.
   // return value prefix_matched is set to true if the offset is confirmed
   // for a key with the same prefix as target.
   Status GetOffset(const Slice& target, const Slice& prefix,
                    uint32_t prefix_hash, bool& prefix_matched,
-                   uint32_t* ret_offset) const;
+                   uint32_t* offset) const;
 
   Slice GetUserKey(const Slice& key) const {
     return Slice(key.data(), key.size() - 8);