Use base::flat_set in place of std::set when merging policies.

This is an attempt to reduce heap usage in merges.

BUG: 982452
Change-Id: If17dad64ab8adde29384765720c210e7f5e72601
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1762083Reviewed-by: Owen Min <zmin@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688618}

components/policy/core/common/policy_merger.cc

@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <array>
 #include <map>
+#include <set>
 
 #include "components/policy/core/common/policy_merger.h"
 #include "components/policy/core/common/policy_pref_names.h"
@@ -11,14 +13,19 @@
 
 namespace policy {
 
-const char* const kDictionaryPoliciesToMerge[] = {
+namespace {
+
+constexpr std::array<const char*, 7> kDictionaryPoliciesToMerge{
     key::kContentPackManualBehaviorURLs,
     key::kExtensionSettings,
     key::kDeviceLoginScreenPowerManagement,
     key::kKeyPermissions,
     key::kPowerManagementIdleSettings,
     key::kScreenBrightnessPercent,
-    key::kScreenLockDelays};
+    key::kScreenLockDelays,
+};
+
+}  // namespace
 
 // static
 bool PolicyMerger::ConflictCanBeMerged(const PolicyMap::Entry& conflict,
@@ -39,7 +46,7 @@ PolicyMerger::PolicyMerger() = default;
 PolicyMerger::~PolicyMerger() = default;
 
 PolicyListMerger::PolicyListMerger(
-    const std::set<std::string> policies_to_merge)
+    base::flat_set<std::string> policies_to_merge)
     : policies_to_merge_(std::move(policies_to_merge)) {}
 PolicyListMerger::~PolicyListMerger() = default;
 
@@ -120,10 +127,10 @@ void PolicyListMerger::DoMerge(PolicyMap::Entry* policy) const {
 }
 
 PolicyDictionaryMerger::PolicyDictionaryMerger(
-    std::set<std::string> policies_to_merge)
+    base::flat_set<std::string> policies_to_merge)
     : policies_to_merge_(std::move(policies_to_merge)),
-      allowed_policies_(std::begin(kDictionaryPoliciesToMerge),
-                        std::end(kDictionaryPoliciesToMerge)) {}
+      allowed_policies_(kDictionaryPoliciesToMerge.begin(),
+                        kDictionaryPoliciesToMerge.end()) {}
 PolicyDictionaryMerger::~PolicyDictionaryMerger() = default;
 
 void PolicyDictionaryMerger::Merge(PolicyMap::PolicyMapType* policies) const {
@@ -135,7 +142,7 @@ void PolicyDictionaryMerger::Merge(PolicyMap::PolicyMapType* policies) const {
 }
 
 void PolicyDictionaryMerger::SetAllowedPoliciesForTesting(
-    std::set<std::string> allowed_policies) {
+    base::flat_set<std::string> allowed_policies) {
   allowed_policies_ = std::move(allowed_policies);
 }
 

components/policy/core/common/policy_merger.h

@@ -7,18 +7,16 @@
 
 #include <stddef.h>
 #include <memory>
-#include <set>
 #include <string>
 #include <vector>
 
+#include "base/containers/flat_set.h"
 #include "base/macros.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/policy_export.h"
 
 namespace policy {
 
-extern const char* const kDictionaryPoliciesToMerge[];
-
 // Abstract class that provides an interface to apply custom merging logic on a
 // set of policies.
 class POLICY_EXPORT PolicyMerger {
@@ -35,7 +33,7 @@ class POLICY_EXPORT PolicyMerger {
 // multiple sources concatenated without duplicates.
 class POLICY_EXPORT PolicyListMerger : public PolicyMerger {
  public:
-  explicit PolicyListMerger(std::set<std::string> policies_to_merge);
+  explicit PolicyListMerger(base::flat_set<std::string> policies_to_merge);
   ~PolicyListMerger() override;
 
   // Merges the list policies from |policies| that have multiple sources.
@@ -52,7 +50,7 @@ class POLICY_EXPORT PolicyListMerger : public PolicyMerger {
   // remain unchanged if there is nothing to merge.
   void DoMerge(PolicyMap::Entry* policy) const;
 
-  const std::set<std::string> policies_to_merge_;
+  const base::flat_set<std::string> policies_to_merge_;
 
   DISALLOW_COPY_AND_ASSIGN(PolicyListMerger);
 };
@@ -63,12 +61,14 @@ class POLICY_EXPORT PolicyListMerger : public PolicyMerger {
 // using the key coming from the highest priority source.
 class POLICY_EXPORT PolicyDictionaryMerger : public PolicyMerger {
  public:
-  explicit PolicyDictionaryMerger(std::set<std::string> policies_to_merge);
+  explicit PolicyDictionaryMerger(
+      base::flat_set<std::string> policies_to_merge);
   ~PolicyDictionaryMerger() override;
 
   // Merges the dictionary policies from |policies| that have multiple sources.
   void Merge(PolicyMap::PolicyMapType* policies) const override;
-  void SetAllowedPoliciesForTesting(std::set<std::string> allowed_policies);
+  void SetAllowedPoliciesForTesting(
+      base::flat_set<std::string> allowed_policies);
 
  private:
   // Returns True if |policy_name| is in the list of policies to merge and if
@@ -81,8 +81,8 @@ class POLICY_EXPORT PolicyDictionaryMerger : public PolicyMerger {
   // intact if there is nothing to merge.
   void DoMerge(PolicyMap::Entry* policy) const;
 
-  const std::set<std::string> policies_to_merge_;
-  std::set<std::string> allowed_policies_;
+  const base::flat_set<std::string> policies_to_merge_;
+  base::flat_set<std::string> allowed_policies_;
 
   DISALLOW_COPY_AND_ASSIGN(PolicyDictionaryMerger);
 };

components/policy/core/common/policy_service_impl.cc

@@ -10,6 +10,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/containers/flat_set.h"
 #include "base/feature_list.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
@@ -73,24 +74,35 @@ void RemapProxyPolicies(PolicyMap* policies) {
   }
 }
 
-// Returns a list of string values of |policy|. Returns an empty array if
-// the values are not strings.
-std::set<std::string> GetStringListPolicyItems(const PolicyBundle& bundle,
+// Returns the string values of |policy|. Returns an empty set if the values are
+// not strings.
+base::flat_set<std::string> GetStringListPolicyItems(
+    const PolicyBundle& bundle,
     const PolicyNamespace& space,
     const std::string& policy) {
   const PolicyMap& chrome_policies = bundle.Get(space);
   const base::Value* items_ptr = chrome_policies.GetValue(policy);
 
-  std::set<std::string> items;
+  if (!items_ptr)
+    return base::flat_set<std::string>();
 
-  if (items_ptr) {
-    for (const auto& item : items_ptr->GetList()) {
+  // Count the items to allocate the right-sized vector for them.
+  const auto& item_list = items_ptr->GetList();
+  const auto item_count =
+      std::count_if(item_list.begin(), item_list.end(),
+                    [](const auto& item) { return item.is_string(); });
+
+  // Allocate the storage.
+  std::vector<std::string> item_vector;
+  item_vector.reserve(item_count);
+
+  // Populate it.
+  for (const auto& item : item_list) {
     if (item.is_string())
-        items.emplace(item.GetString());
-    }
+      item_vector.emplace_back(item.GetString());
   }
 
-  return items;
+  return base::flat_set<std::string>(std::move(item_vector));
 }
 
 }  // namespace
@@ -223,10 +235,11 @@ void PolicyServiceImpl::MergeAndTriggerUpdates() {
   }
 
   // Merges all the mergeable policies
-  std::set<std::string> policy_lists_to_merge = GetStringListPolicyItems(
+  base::flat_set<std::string> policy_lists_to_merge = GetStringListPolicyItems(
       bundle, chrome_namespace, key::kPolicyListMultipleSourceMergeList);
-  std::set<std::string> policy_dictionaries_to_merge = GetStringListPolicyItems(
-      bundle, chrome_namespace, key::kPolicyDictionaryMultipleSourceMergeList);
+  base::flat_set<std::string> policy_dictionaries_to_merge =
+      GetStringListPolicyItems(bundle, chrome_namespace,
+                               key::kPolicyDictionaryMultipleSourceMergeList);
 
   auto& chrome_policies = bundle.Get(chrome_namespace);