Add language lookup caching to UlpLanguageCodeLocator.

We add caching so that s2langquadtree lookup is not necessary if the geolocation hasn't changed much. It is done by finding the leaf cell for which we have (or have not) found a language in the quad tree, saving that cell and comparing it against future queries. This is the first part of an effort to reduce memory footprint of the new UlpLanguageCode locator. Second part will have the UlpLanguageCodeLocator construct the quad tree on query instead of on construction. Bug: 929155 Change-Id: I0f5596acf40ff0bdf91941eb026f0b8a83bb7d9d Reviewed-on: https://chromium-review.googlesource.com/c/1459057 Reviewed-by: anthonyvd <anthonyvd@chromium.org> Reviewed-by: Moe Ahmadi <mahmadi@chromium.org> Commit-Queue: Alexandre Frechette <frechette@chromium.org> Cr-Commit-Position: refs/heads/master@{#631746}
Denger-Network · Feb 13, 2019 · 827b146 · 827b146
1 parent 31077fd
commit 827b146
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 27 deletions.
diff --git a/components/language/content/browser/ulp_language_code_locator/s2langquadtree.cc b/components/language/content/browser/ulp_language_code_locator/s2langquadtree.cc
@@ -12,19 +12,20 @@ S2LangQuadTreeNode::S2LangQuadTreeNode(const S2LangQuadTreeNode& other) =
     default;
 S2LangQuadTreeNode::~S2LangQuadTreeNode() = default;
 
-std::string S2LangQuadTreeNode::Get(const S2CellId& cell) const {
+std::string S2LangQuadTreeNode::Get(const S2CellId& cell,
+                                    int* level_ptr) const {
   const S2LangQuadTreeNode* node = this;
-  const std::string* language = &(node->language_);
-  for (int current_level = 1; current_level <= cell.level(); ++current_level) {
-    const S2LangQuadTreeNode* const child =
-        node->GetChild(cell.child_position(current_level));
-    if (child == nullptr)
-      break;
-    node = child;
-    if (!node->language_.empty())
-      language = &(node->language_);
+  for (int current_level = 0; current_level <= cell.level(); current_level++) {
+    if (node == nullptr || !node->language_.empty()) {
+      *level_ptr = current_level;
+      return node == nullptr ? "" : node->language_;
+    }
+
+    if (current_level < cell.level())
+      node = node->GetChild(cell.child_position(current_level + 1));
   }
-  return *language;
+  *level_ptr = -1;
+  return "";
 }
 
 const S2LangQuadTreeNode* S2LangQuadTreeNode::GetChild(

diff --git a/components/language/content/browser/ulp_language_code_locator/s2langquadtree.h b/components/language/content/browser/ulp_language_code_locator/s2langquadtree.h
@@ -21,9 +21,15 @@ class S2LangQuadTreeNode {
   S2LangQuadTreeNode(const S2LangQuadTreeNode& other);
   ~S2LangQuadTreeNode();
 
-  // Return the language in the deepest/lowest/smallest node containing the
-  // given cell.
-  std::string Get(const S2CellId& cell) const;
+  // Return language of the leaf containing the given |cell|.
+  // Empty string if a null-leaf contains given |cell|.
+  // |level_ptr| is set to the level (see S2CellId::level) of the leaf. (-1 if
+  // |cell| matches an internal node).
+  std::string Get(const S2CellId& cell, int* level_ptr) const;
+  std::string Get(const S2CellId& cell) const {
+    int level;
+    return Get(cell, &level);
+  }
 
   // Reconstruct a S2LangQuadTree with structure given by |tree| and with
   // languages given by |languages|. |tree| represents a depth-first traversal

diff --git a/components/language/content/browser/ulp_language_code_locator/s2langquadtree_unittest.cc b/components/language/content/browser/ulp_language_code_locator/s2langquadtree_unittest.cc
@@ -26,54 +26,65 @@ namespace language {
 TEST(S2LangQuadTreeTest, Empty) {
   S2LangQuadTreeNode root;
   const S2CellId cell = S2CellId::FromFace(0);
-  const std::string language = root.Get(cell);
+  int level;
+  const std::string language = root.Get(cell, &level);
   EXPECT_TRUE(language.empty());
+  EXPECT_EQ(level, -1);
 }
 
 TEST(S2LangQuadTreeTest, RootIsLeaf_FaceIsPresent) {
   const std::vector<std::string> languages{"fr"};
   const std::bitset<2> tree("11");  // String is in reverse order.
   const S2LangQuadTreeNode root = GetTree(languages, tree);
   const S2CellId cell = S2CellId::FromFace(0);
-  const std::string language = root.Get(cell);
+  int level;
+  const std::string language = root.Get(cell, &level);
   EXPECT_EQ(language, "fr");
+  EXPECT_EQ(level, 0);
 }
 
 TEST(S2LangQuadTreeTest, RootIsLeaf_FaceChildGetsFaceLanguage) {
   const std::vector<std::string> languages{"fr"};
   const std::bitset<2> tree("11");  // String is in reverse order.
   const S2LangQuadTreeNode root = GetTree(languages, tree);
   const S2CellId cell = S2CellId::FromFace(0).child(0);
-  const std::string language = root.Get(cell);
+  int level;
+  const std::string language = root.Get(cell, &level);
   EXPECT_EQ(language, "fr");
+  EXPECT_EQ(level, 0);
 }
 
 TEST(S2LangQuadTreeTest, RootThenSingleLeaf_LeafIsPresent) {
   const std::vector<std::string> languages{"fr"};
   const std::bitset<9> tree("110101010");  // String is in reverse order.
   const S2LangQuadTreeNode root = GetTree(languages, tree);
   const S2CellId cell = S2CellId::FromFace(0).child(3);
-  const std::string language = root.Get(cell);
+  int level;
+  const std::string language = root.Get(cell, &level);
   EXPECT_EQ(language, "fr");
+  EXPECT_EQ(level, 1);
 }
 
 TEST(S2LangQuadTreeTest, RootThenSingleLeaf_ParentIsAbsent) {
   const std::vector<std::string> languages{"fr"};
   const std::bitset<9> tree("110101010");  // String is in reverse order.
   const S2LangQuadTreeNode root = GetTree(languages, tree);
   const S2CellId cell = S2CellId::FromFace(0);
-  const std::string language = root.Get(cell);
-  LOG(INFO) << language;
+  int level;
+  const std::string language = root.Get(cell, &level);
   EXPECT_TRUE(language.empty());
+  EXPECT_EQ(level, -1);
 }
 
 TEST(S2LangQuadTreeTest, RootThenSingleLeaf_SiblingIsAbsent) {
   const std::vector<std::string> languages{"fr"};
   const std::bitset<9> tree("110101010");  // String is in reverse order.
   const S2LangQuadTreeNode root = GetTree(languages, tree);
   const S2CellId cell = S2CellId::FromFace(0).child(0);
-  const std::string language = root.Get(cell);
+  int level;
+  const std::string language = root.Get(cell, &level);
   EXPECT_TRUE(language.empty());
+  EXPECT_EQ(level, 1);
 }
 
 TEST(S2LangQuadTreeTest, RootThenAllLeaves_LeavesArePresent) {
@@ -82,21 +93,27 @@ TEST(S2LangQuadTreeTest, RootThenAllLeaves_LeavesArePresent) {
   const S2LangQuadTreeNode root = GetTree(languages, tree);
   for (int leaf_index = 0; leaf_index < 3; leaf_index++) {
     const S2CellId cell = S2CellId::FromFace(0).child(leaf_index);
-    const std::string language = root.Get(cell);
+    int level;
+    const std::string language = root.Get(cell, &level);
     EXPECT_EQ(language, "fr");
+    EXPECT_EQ(level, 1);
   }
   const S2CellId cell = S2CellId::FromFace(0).child(3);
-  const std::string language = root.Get(cell);
+  int level;
+  const std::string language = root.Get(cell, &level);
   EXPECT_EQ(language, "en");
+  EXPECT_EQ(level, 1);
 }
 
 TEST(S2LangQuadTreeTest, RootThenAllLeaves_ParentIsAbsent) {
   const std::vector<std::string> languages{"fr", "en"};
   const std::bitset<13> tree("0111011011010");  // String is in reverse order.
   const S2LangQuadTreeNode root = GetTree(languages, tree);
   const S2CellId cell = S2CellId::FromFace(0);
-  const std::string language = root.Get(cell);
+  int level;
+  const std::string language = root.Get(cell, &level);
   EXPECT_TRUE(language.empty());
+  EXPECT_EQ(level, -1);
 }
 
 }  // namespace language
diff --git a/components/language/content/browser/ulp_language_code_locator/ulp_language_code_locator.cc b/components/language/content/browser/ulp_language_code_locator/ulp_language_code_locator.cc
@@ -12,9 +12,15 @@
 
 namespace language {
 
+struct UlpLanguageCodeLocator::CellLanguagePair {
+  S2CellId cell = S2CellId::None();
+  std::string language = "";
+};
+
 UlpLanguageCodeLocator::UlpLanguageCodeLocator(
     std::vector<std::unique_ptr<S2LangQuadTreeNode>>&& roots) {
   roots_ = std::move(roots);
+  cache_ = std::vector<CellLanguagePair>(roots_.size());
 }
 
 UlpLanguageCodeLocator::~UlpLanguageCodeLocator() {}
@@ -24,8 +30,22 @@ std::vector<std::string> UlpLanguageCodeLocator::GetLanguageCodes(
     double longitude) const {
   S2CellId cell(S2LatLng::FromDegrees(latitude, longitude));
   std::vector<std::string> languages;
-  for (const auto& root : roots_) {
-    const std::string language = root.get()->Get(cell);
+  for (size_t index = 0; index < roots_.size(); index++) {
+    CellLanguagePair cached = cache_[index];
+    std::string language;
+    if (cached.cell.is_valid() && cached.cell.contains(cell)) {
+      language = cached.language;
+    } else {
+      const auto& root = roots_[index];
+      int level;
+      language = root.get()->Get(cell, &level);
+      if (level != -1) {
+        //|cell|.parent(|level|) is the ancestor S2Cell of |cell| for which
+        // there's a matching language in the tree.
+        cache_[index].cell = cell.parent(level);
+        cache_[index].language = language;
+      }
+    }
     if (!language.empty())
       languages.push_back(language);
   }

diff --git a/components/language/content/browser/ulp_language_code_locator/ulp_language_code_locator.h b/components/language/content/browser/ulp_language_code_locator/ulp_language_code_locator.h
@@ -31,6 +31,8 @@ class UlpLanguageCodeLocator : public LanguageCodeLocator {
 
  private:
   std::vector<std::unique_ptr<S2LangQuadTreeNode>> roots_;
+  struct CellLanguagePair;
+  mutable std::vector<CellLanguagePair> cache_;
   DISALLOW_COPY_AND_ASSIGN(UlpLanguageCodeLocator);
 };
 }  // namespace language

diff --git a/.../language/content/browser/ulp_language_code_locator/ulp_language_code_locator_unittest.cc b/.../language/content/browser/ulp_language_code_locator/ulp_language_code_locator_unittest.cc
@@ -49,7 +49,7 @@ void ExpectLatLngHasLanguages(const UlpLanguageCodeLocator& locator,
   EXPECT_THAT(languages, ::testing::ElementsAreArray(languages_expected));
 }
 
-TEST(UlpLanguageCodeLocatorTest, QuadrantMatchLanguages) {
+TEST(UlpLanguageCodeLocatorTest, TreeLeaves) {
   std::vector<std::unique_ptr<S2LangQuadTreeNode>> roots = GetTestTrees();
   const UlpLanguageCodeLocator locator(std::move(roots));
   const S2CellId face = S2CellId::FromFace(0);
@@ -59,4 +59,32 @@ TEST(UlpLanguageCodeLocatorTest, QuadrantMatchLanguages) {
   ExpectLatLngHasLanguages(locator, face.child(2), {"fr", "en"});
   ExpectLatLngHasLanguages(locator, face.child(3), {"en", "en"});
 }
+
+TEST(UlpLanguageCodeLocatorTest, Idempotence) {
+  std::vector<std::unique_ptr<S2LangQuadTreeNode>> roots = GetTestTrees();
+  const UlpLanguageCodeLocator locator(std::move(roots));
+  const S2CellId face = S2CellId::FromFace(0);
+
+  ExpectLatLngHasLanguages(locator, face.child(0), {"fr", "de"});
+  ExpectLatLngHasLanguages(locator, face.child(0), {"fr", "de"});
+
+  ExpectLatLngHasLanguages(locator, face.child(3), {"en", "en"});
+  ExpectLatLngHasLanguages(locator, face.child(3), {"en", "en"});
+}
+
+TEST(UlpLanguageCodeLocatorTest, TreeLeafDescendants) {
+  std::vector<std::unique_ptr<S2LangQuadTreeNode>> roots = GetTestTrees();
+  const UlpLanguageCodeLocator locator(std::move(roots));
+  const S2CellId cell = S2CellId::FromFace(0).child(0);
+
+  ExpectLatLngHasLanguages(locator, cell, {"fr", "de"});
+
+  const int depth = 2;
+  // Check that the 4**|depth| descendants of |child| map to the same language.
+  const int level = cell.level() + depth;
+  for (S2CellId descendant = cell.child_begin(level);
+       descendant != cell.child_end(level); descendant = descendant.next())
+    ExpectLatLngHasLanguages(locator, descendant, {"fr", "de"});
+}
+
 }  // namespace language