Update Icing from upstream.

Descriptions:
========================================================================
Add flag to guard PersistentHashMapKeyMapper and DynamicTrieKeyMapper
========================================================================
Add flag to guard FileBackedVector premapping
========================================================================
Check tokenizer type when assigning section id
========================================================================

Bug:193919210

Change-Id: I682e22d4ba647220b7222fcc63c41bd0e01bbeb9

4 files changed, 85 insertions, 58 deletions

diff --git a/icing/store/key-mapper_benchmark.cc b/icing/store/key-mapper_benchmark.cc
index 1ce54c7..c25fe30 100644
--- a/icing/store/key-mapper_benchmark.cc
+++ b/icing/store/key-mapper_benchmark.cc
@@ -82,8 +82,8 @@ class KeyMapperBenchmark {
     std::string working_path =
         absl_ports::StrCat(base_dir, "/", "key_mapper_dir");
     return PersistentHashMapKeyMapper<int>::Create(
-        filesystem, std::move(working_path), max_num_entries,
-        /*average_kv_byte_size=*/kKeyLength + 1 + sizeof(int),
+        filesystem, std::move(working_path), /*pre_mapping_fbv=*/true,
+        max_num_entries, /*average_kv_byte_size=*/kKeyLength + 1 + sizeof(int),
         /*max_load_factor_percent=*/100);
   }
 
diff --git a/icing/store/key-mapper_test.cc b/icing/store/key-mapper_test.cc
index 1367c2d..fa7d1e8 100644
--- a/icing/store/key-mapper_test.cc
+++ b/icing/store/key-mapper_test.cc
@@ -16,14 +16,12 @@
 
 #include <memory>
 #include <string>
-#include <type_traits>
 #include <unordered_map>
 
 #include "icing/text_classifier/lib3/utils/base/status.h"
 #include "icing/text_classifier/lib3/utils/base/statusor.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
-#include "icing/absl_ports/canonical_errors.h"
 #include "icing/file/filesystem.h"
 #include "icing/store/document-id.h"
 #include "icing/store/dynamic-trie-key-mapper.h"
@@ -43,11 +41,23 @@ namespace {
 
 constexpr int kMaxDynamicTrieKeyMapperSize = 3 * 1024 * 1024;  // 3 MiB
 
-template <typename T>
-class KeyMapperTest : public ::testing::Test {
- protected:
-  using KeyMapperType = T;
+enum class KeyMapperType {
+  kDynamicTrie,
+  kPersistentHashMap,
+};
+
+struct KeyMapperTestParam {
+  KeyMapperType key_mapper_type;
+  bool pre_mapping_fbv;
 
+  explicit KeyMapperTestParam(KeyMapperType key_mapper_type_in,
+                              bool pre_mapping_fbv_in)
+      : key_mapper_type(key_mapper_type_in),
+        pre_mapping_fbv(pre_mapping_fbv_in) {}
+};
+
+class KeyMapperTest : public ::testing::TestWithParam<KeyMapperTestParam> {
+ protected:
   void SetUp() override {
     base_dir_ = GetTestTempDir() + "/icing";
     ASSERT_THAT(filesystem_.CreateDirectoryRecursively(base_dir_.c_str()),
@@ -60,24 +70,29 @@ class KeyMapperTest : public ::testing::Test {
     filesystem_.DeleteDirectoryRecursively(base_dir_.c_str());
   }
 
-  template <typename UnknownKeyMapperType>
   libtextclassifier3::StatusOr<std::unique_ptr<KeyMapper<DocumentId>>>
   CreateKeyMapper() {
-    return absl_ports::InvalidArgumentError("Unknown type");
+    const KeyMapperTestParam& param = GetParam();
+    switch (param.key_mapper_type) {
+      case KeyMapperType::kDynamicTrie:
+        return DynamicTrieKeyMapper<DocumentId>::Create(
+            filesystem_, working_dir_, kMaxDynamicTrieKeyMapperSize);
+      case KeyMapperType::kPersistentHashMap:
+        return PersistentHashMapKeyMapper<DocumentId>::Create(
+            filesystem_, working_dir_, param.pre_mapping_fbv);
+    }
   }
 
-  template <>
-  libtextclassifier3::StatusOr<std::unique_ptr<KeyMapper<DocumentId>>>
-  CreateKeyMapper<DynamicTrieKeyMapper<DocumentId>>() {
-    return DynamicTrieKeyMapper<DocumentId>::Create(
-        filesystem_, working_dir_, kMaxDynamicTrieKeyMapperSize);
-  }
-
-  template <>
-  libtextclassifier3::StatusOr<std::unique_ptr<KeyMapper<DocumentId>>>
-  CreateKeyMapper<PersistentHashMapKeyMapper<DocumentId>>() {
-    return PersistentHashMapKeyMapper<DocumentId>::Create(filesystem_,
-                                                          working_dir_);
+  libtextclassifier3::Status DeleteKeyMapper() {
+    const KeyMapperTestParam& param = GetParam();
+    switch (param.key_mapper_type) {
+      case KeyMapperType::kDynamicTrie:
+        return DynamicTrieKeyMapper<DocumentId>::Delete(filesystem_,
+                                                        working_dir_);
+      case KeyMapperType::kPersistentHashMap:
+        return PersistentHashMapKeyMapper<DocumentId>::Delete(filesystem_,
+                                                              working_dir_);
+    }
   }
 
   std::string base_dir_;
@@ -85,10 +100,6 @@ class KeyMapperTest : public ::testing::Test {
   Filesystem filesystem_;
 };
 
-using TestTypes = ::testing::Types<DynamicTrieKeyMapper<DocumentId>,
-                                   PersistentHashMapKeyMapper<DocumentId>>;
-TYPED_TEST_SUITE(KeyMapperTest, TestTypes);
-
 std::unordered_map<std::string, DocumentId> GetAllKeyValuePairs(
     const KeyMapper<DocumentId>* key_mapper) {
   std::unordered_map<std::string, DocumentId> ret;
@@ -101,15 +112,15 @@ std::unordered_map<std::string, DocumentId> GetAllKeyValuePairs(
   return ret;
 }
 
-TYPED_TEST(KeyMapperTest, CreateNewKeyMapper) {
+TEST_P(KeyMapperTest, CreateNewKeyMapper) {
   ICING_ASSERT_OK_AND_ASSIGN(std::unique_ptr<KeyMapper<DocumentId>> key_mapper,
-                             this->template CreateKeyMapper<TypeParam>());
+                             CreateKeyMapper());
   EXPECT_THAT(key_mapper->num_keys(), 0);
 }
 
-TYPED_TEST(KeyMapperTest, CanUpdateSameKeyMultipleTimes) {
+TEST_P(KeyMapperTest, CanUpdateSameKeyMultipleTimes) {
   ICING_ASSERT_OK_AND_ASSIGN(std::unique_ptr<KeyMapper<DocumentId>> key_mapper,
-                             this->template CreateKeyMapper<TypeParam>());
+                             CreateKeyMapper());
 
   ICING_EXPECT_OK(key_mapper->Put("default-google.com", 100));
   ICING_EXPECT_OK(key_mapper->Put("default-youtube.com", 50));
@@ -125,9 +136,9 @@ TYPED_TEST(KeyMapperTest, CanUpdateSameKeyMultipleTimes) {
   EXPECT_THAT(key_mapper->num_keys(), 2);
 }
 
-TYPED_TEST(KeyMapperTest, GetOrPutOk) {
+TEST_P(KeyMapperTest, GetOrPutOk) {
   ICING_ASSERT_OK_AND_ASSIGN(std::unique_ptr<KeyMapper<DocumentId>> key_mapper,
-                             this->template CreateKeyMapper<TypeParam>());
+                             CreateKeyMapper());
 
   EXPECT_THAT(key_mapper->Get("foo"),
               StatusIs(libtextclassifier3::StatusCode::NOT_FOUND));
@@ -135,9 +146,9 @@ TYPED_TEST(KeyMapperTest, GetOrPutOk) {
   EXPECT_THAT(key_mapper->Get("foo"), IsOkAndHolds(1));
 }
 
-TYPED_TEST(KeyMapperTest, CanPersistToDiskRegularly) {
+TEST_P(KeyMapperTest, CanPersistToDiskRegularly) {
   ICING_ASSERT_OK_AND_ASSIGN(std::unique_ptr<KeyMapper<DocumentId>> key_mapper,
-                             this->template CreateKeyMapper<TypeParam>());
+                             CreateKeyMapper());
 
   // Can persist an empty DynamicTrieKeyMapper.
   ICING_EXPECT_OK(key_mapper->PersistToDisk());
@@ -160,16 +171,15 @@ TYPED_TEST(KeyMapperTest, CanPersistToDiskRegularly) {
   EXPECT_THAT(key_mapper->num_keys(), 2);
 }
 
-TYPED_TEST(KeyMapperTest, CanUseAcrossMultipleInstances) {
+TEST_P(KeyMapperTest, CanUseAcrossMultipleInstances) {
   ICING_ASSERT_OK_AND_ASSIGN(std::unique_ptr<KeyMapper<DocumentId>> key_mapper,
-                             this->template CreateKeyMapper<TypeParam>());
+                             CreateKeyMapper());
   ICING_EXPECT_OK(key_mapper->Put("default-google.com", 100));
   ICING_EXPECT_OK(key_mapper->PersistToDisk());
 
   key_mapper.reset();
 
-  ICING_ASSERT_OK_AND_ASSIGN(key_mapper,
-                             this->template CreateKeyMapper<TypeParam>());
+  ICING_ASSERT_OK_AND_ASSIGN(key_mapper, CreateKeyMapper());
   EXPECT_THAT(key_mapper->num_keys(), 1);
   EXPECT_THAT(key_mapper->Get("default-google.com"), IsOkAndHolds(100));
 
@@ -181,29 +191,26 @@ TYPED_TEST(KeyMapperTest, CanUseAcrossMultipleInstances) {
   EXPECT_THAT(key_mapper->Get("default-google.com"), IsOkAndHolds(300));
 }
 
-TYPED_TEST(KeyMapperTest, CanDeleteAndRestartKeyMapping) {
+TEST_P(KeyMapperTest, CanDeleteAndRestartKeyMapping) {
   // Can delete even if there's nothing there
-  ICING_EXPECT_OK(TestFixture::KeyMapperType::Delete(this->filesystem_,
-                                                     this->working_dir_));
+  ICING_EXPECT_OK(DeleteKeyMapper());
 
   ICING_ASSERT_OK_AND_ASSIGN(std::unique_ptr<KeyMapper<DocumentId>> key_mapper,
-                             this->template CreateKeyMapper<TypeParam>());
+                             CreateKeyMapper());
   ICING_EXPECT_OK(key_mapper->Put("default-google.com", 100));
   ICING_EXPECT_OK(key_mapper->PersistToDisk());
-  ICING_EXPECT_OK(TestFixture::KeyMapperType::Delete(this->filesystem_,
-                                                     this->working_dir_));
+  ICING_EXPECT_OK(DeleteKeyMapper());
 
   key_mapper.reset();
-  ICING_ASSERT_OK_AND_ASSIGN(key_mapper,
-                             this->template CreateKeyMapper<TypeParam>());
+  ICING_ASSERT_OK_AND_ASSIGN(key_mapper, CreateKeyMapper());
   EXPECT_THAT(key_mapper->num_keys(), 0);
   ICING_EXPECT_OK(key_mapper->Put("default-google.com", 100));
   EXPECT_THAT(key_mapper->num_keys(), 1);
 }
 
-TYPED_TEST(KeyMapperTest, Iterator) {
+TEST_P(KeyMapperTest, Iterator) {
   ICING_ASSERT_OK_AND_ASSIGN(std::unique_ptr<KeyMapper<DocumentId>> key_mapper,
-                             this->template CreateKeyMapper<TypeParam>());
+                             CreateKeyMapper());
   EXPECT_THAT(GetAllKeyValuePairs(key_mapper.get()), IsEmpty());
 
   ICING_EXPECT_OK(key_mapper->Put("foo", /*value=*/1));
@@ -217,6 +224,15 @@ TYPED_TEST(KeyMapperTest, Iterator) {
       UnorderedElementsAre(Pair("foo", 1), Pair("bar", 2), Pair("baz", 3)));
 }
 
+INSTANTIATE_TEST_SUITE_P(
+    KeyMapperTest, KeyMapperTest,
+    testing::Values(KeyMapperTestParam(KeyMapperType::kDynamicTrie,
+                                       /*pre_mapping_fbv_in=*/true),
+                    KeyMapperTestParam(KeyMapperType::kPersistentHashMap,
+                                       /*pre_mapping_fbv_in=*/true),
+                    KeyMapperTestParam(KeyMapperType::kPersistentHashMap,
+                                       /*pre_mapping_fbv_in=*/false)));
+
 }  // namespace
 
 }  // namespace lib
diff --git a/icing/store/persistent-hash-map-key-mapper.h b/icing/store/persistent-hash-map-key-mapper.h
index 5f83e6f..0596fe3 100644
--- a/icing/store/persistent-hash-map-key-mapper.h
+++ b/icing/store/persistent-hash-map-key-mapper.h
@@ -38,6 +38,13 @@ namespace lib {
 template <typename T, typename Formatter = absl_ports::DefaultFormatter>
 class PersistentHashMapKeyMapper : public KeyMapper<T, Formatter> {
  public:
+  static constexpr int32_t kDefaultMaxNumEntries =
+      PersistentHashMap::Entry::kMaxNumEntries;
+  static constexpr int32_t kDefaultAverageKVByteSize =
+      PersistentHashMap::Options::kDefaultAverageKVByteSize;
+  static constexpr int32_t kDefaultMaxLoadFactorPercent =
+      PersistentHashMap::Options::kDefaultMaxLoadFactorPercent;
+
   // Returns an initialized instance of PersistentHashMapKeyMapper that can
   // immediately handle read/write operations.
   // Returns any encountered IO errors.
@@ -50,6 +57,9 @@ class PersistentHashMapKeyMapper : public KeyMapper<T, Formatter> {
   //               PersistentHashMapKeyMapper would be created. See
   //               PersistentStorage for more details about the concept of
   //               working_path.
+  // pre_mapping_fbv: flag indicating whether memory map max possible file size
+  //                  for underlying FileBackedVector before growing the actual
+  //                  file size.
   // max_num_entries: max # of kvps. It will be used to compute 3 storages size.
   // average_kv_byte_size: average byte size of a single key + serialized value.
   //                       It will be used to compute kv_storage size.
@@ -63,11 +73,9 @@ class PersistentHashMapKeyMapper : public KeyMapper<T, Formatter> {
   static libtextclassifier3::StatusOr<
       std::unique_ptr<PersistentHashMapKeyMapper<T, Formatter>>>
   Create(const Filesystem& filesystem, std::string working_path,
-         int32_t max_num_entries = PersistentHashMap::Entry::kMaxNumEntries,
-         int32_t average_kv_byte_size =
-             PersistentHashMap::Options::kDefaultAverageKVByteSize,
-         int32_t max_load_factor_percent =
-             PersistentHashMap::Options::kDefaultMaxLoadFactorPercent);
+         bool pre_mapping_fbv, int32_t max_num_entries = kDefaultMaxNumEntries,
+         int32_t average_kv_byte_size = kDefaultAverageKVByteSize,
+         int32_t max_load_factor_percent = kDefaultMaxLoadFactorPercent);
 
   // Deletes working_path (and all the files under it recursively) associated
   // with the PersistentHashMapKeyMapper.
@@ -166,7 +174,7 @@ template <typename T, typename Formatter>
     std::unique_ptr<PersistentHashMapKeyMapper<T, Formatter>>>
 PersistentHashMapKeyMapper<T, Formatter>::Create(
     const Filesystem& filesystem, std::string working_path,
-    int32_t max_num_entries, int32_t average_kv_byte_size,
+    bool pre_mapping_fbv, int32_t max_num_entries, int32_t average_kv_byte_size,
     int32_t max_load_factor_percent) {
   ICING_ASSIGN_OR_RETURN(
       std::unique_ptr<PersistentHashMap> persistent_hash_map,
@@ -176,7 +184,10 @@ PersistentHashMapKeyMapper<T, Formatter>::Create(
               /*value_type_size_in=*/sizeof(T),
               /*max_num_entries_in=*/max_num_entries,
               /*max_load_factor_percent_in=*/max_load_factor_percent,
-              /*average_kv_byte_size_in=*/average_kv_byte_size)));
+              /*average_kv_byte_size_in=*/average_kv_byte_size,
+              /*init_num_buckets_in=*/
+              PersistentHashMap::Options::kDefaultInitNumBuckets,
+              /*pre_mapping_fbv_in=*/pre_mapping_fbv)));
   return std::unique_ptr<PersistentHashMapKeyMapper<T, Formatter>>(
       new PersistentHashMapKeyMapper<T, Formatter>(
           std::move(persistent_hash_map)));
diff --git a/icing/store/persistent-hash-map-key-mapper_test.cc b/icing/store/persistent-hash-map-key-mapper_test.cc
index c937c43..0d610e9 100644
--- a/icing/store/persistent-hash-map-key-mapper_test.cc
+++ b/icing/store/persistent-hash-map-key-mapper_test.cc
@@ -41,9 +41,9 @@ class PersistentHashMapKeyMapperTest : public testing::Test {
 };
 
 TEST_F(PersistentHashMapKeyMapperTest, InvalidBaseDir) {
-  EXPECT_THAT(
-      PersistentHashMapKeyMapper<DocumentId>::Create(filesystem_, "/dev/null"),
-      StatusIs(libtextclassifier3::StatusCode::INTERNAL));
+  EXPECT_THAT(PersistentHashMapKeyMapper<DocumentId>::Create(
+                  filesystem_, "/dev/null", /*pre_mapping_fbv=*/false),
+              StatusIs(libtextclassifier3::StatusCode::INTERNAL));
 }
 
 }  // namespace