Commit 8d01c025 authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

[zxcvbn] Use base::flat_map for RankedDicts

This change uses base::flat_map instead of std::unordered_map in
zxcvbn's RankedDicts in order to reduce memory usage.

Since a base::flat_map requires its elements to be move assignable,
the mapped_type is changed from a reference to a pointer.

Bug: 1115878
Change-Id: I97eada4b85bfbbf0b47a9a73c6c9219f112380c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352735
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: default avatarJoshua Pawlicki <waffles@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Auto-Submit: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798064}
parent b7582667
...@@ -72,7 +72,7 @@ constexpr std::array<TagAndFileName, 6> kTagAndFileNamePairs = {{ ...@@ -72,7 +72,7 @@ constexpr std::array<TagAndFileName, 6> kTagAndFileNamePairs = {{
}}; }};
using RankedDictionaries = using RankedDictionaries =
std::unordered_map<zxcvbn::DictionaryTag, zxcvbn::RankedDict>; base::flat_map<zxcvbn::DictionaryTag, zxcvbn::RankedDict>;
RankedDictionaries ParseRankedDictionaries(const base::FilePath& install_dir) { RankedDictionaries ParseRankedDictionaries(const base::FilePath& install_dir) {
RankedDictionaries result; RankedDictionaries result;
for (const auto& pair : kTagAndFileNamePairs) { for (const auto& pair : kTagAndFileNamePairs) {
......
...@@ -18,6 +18,7 @@ namespace component_updater { ...@@ -18,6 +18,7 @@ namespace component_updater {
namespace { namespace {
using ::testing::Pair; using ::testing::Pair;
using ::testing::Pointee;
using ::testing::UnorderedElementsAre; using ::testing::UnorderedElementsAre;
} // namespace } // namespace
...@@ -132,18 +133,22 @@ TEST_F(ZxcvbnDataComponentInstallerPolicyTest, ComponentReady) { ...@@ -132,18 +133,22 @@ TEST_F(ZxcvbnDataComponentInstallerPolicyTest, ComponentReady) {
zxcvbn::default_ranked_dicts(), zxcvbn::default_ranked_dicts(),
UnorderedElementsAre( UnorderedElementsAre(
Pair(zxcvbn::DictionaryTag::ENGLISH_WIKIPEDIA, Pair(zxcvbn::DictionaryTag::ENGLISH_WIKIPEDIA,
UnorderedElementsAre(Pair("english", 1), Pair("wikipedia", 2))), Pointee(UnorderedElementsAre(Pair("english", 1),
Pair("wikipedia", 2)))),
Pair(zxcvbn::DictionaryTag::FEMALE_NAMES, Pair(zxcvbn::DictionaryTag::FEMALE_NAMES,
UnorderedElementsAre(Pair("female", 1), Pair("names", 2))), Pointee(
Pair(zxcvbn::DictionaryTag::MALE_NAMES, UnorderedElementsAre(Pair("female", 1), Pair("names", 2)))),
UnorderedElementsAre(Pair("male", 1), Pair("names", 2))), Pair(
zxcvbn::DictionaryTag::MALE_NAMES,
Pointee(UnorderedElementsAre(Pair("male", 1), Pair("names", 2)))),
Pair(zxcvbn::DictionaryTag::PASSWORDS, Pair(zxcvbn::DictionaryTag::PASSWORDS,
UnorderedElementsAre(Pair("passwords", 1))), Pointee(UnorderedElementsAre(Pair("passwords", 1)))),
Pair(zxcvbn::DictionaryTag::SURNAMES, Pair(zxcvbn::DictionaryTag::SURNAMES,
UnorderedElementsAre(Pair("surnames", 1))), Pointee(UnorderedElementsAre(Pair("surnames", 1)))),
Pair(zxcvbn::DictionaryTag::US_TV_AND_FILM, Pair(
UnorderedElementsAre(Pair("us", 1), Pair("tv", 2), zxcvbn::DictionaryTag::US_TV_AND_FILM,
Pair("and", 3), Pair("film", 4))))); Pointee(UnorderedElementsAre(Pair("us", 1), Pair("tv", 2),
Pair("and", 3), Pair("film", 4))))));
} }
} // namespace component_updater } // namespace component_updater
include_rules = [ include_rules = [
"+base/containers/flat_map.h",
"+base/no_destructor.h", "+base/no_destructor.h",
"+base/strings", "+base/strings",
"+testing/gmock", "+testing/gmock",
......
...@@ -31,6 +31,8 @@ Applied the following patches: ...@@ -31,6 +31,8 @@ Applied the following patches:
UTF8 API. UTF8 API.
- patches/fix_overflow.diff: Fixes a bug inside scoring.cpp triggered when the - patches/fix_overflow.diff: Fixes a bug inside scoring.cpp triggered when the
number of computed guesses exceeds DBL_MAX. number of computed guesses exceeds DBL_MAX.
- patches/flat_map_ranked.diff: Use base::flat_map instead of std::unordered_map
for ranked dictionaries to reduce memory foot print.
- Dropped the data dictionary from the check-out, as the data will now be - Dropped the data dictionary from the check-out, as the data will now be
provided via component updater. The used dictionaries can be found at provided via component updater. The used dictionaries can be found at
https://github.com/rianhunter/zxcvbn-cpp/tree/cf092c952cd2325ce390b2691231a8f1cb195d59/data https://github.com/rianhunter/zxcvbn-cpp/tree/cf092c952cd2325ce390b2691231a8f1cb195d59/data
......
#include <zxcvbn/frequency_lists.hpp> #include <zxcvbn/frequency_lists.hpp>
#include <unordered_map>
#include <utility> #include <utility>
#include "base/containers/flat_map.h"
#include "base/no_destructor.h" #include "base/no_destructor.h"
namespace zxcvbn { namespace zxcvbn {
namespace { namespace {
std::unordered_map<DictionaryTag, RankedDict>& ranked_dicts() { base::flat_map<DictionaryTag, RankedDict>& ranked_dicts() {
static base::NoDestructor<std::unordered_map<DictionaryTag, RankedDict>> static base::NoDestructor<base::flat_map<DictionaryTag, RankedDict>>
ranked_dicts; ranked_dicts;
return *ranked_dicts; return *ranked_dicts;
} }
} // namespace } // namespace
void SetRankedDicts(std::unordered_map<DictionaryTag, RankedDict> dicts) { void SetRankedDicts(base::flat_map<DictionaryTag, RankedDict> dicts) {
ranked_dicts() = std::move(dicts); ranked_dicts() = std::move(dicts);
} }
RankedDicts convert_to_ranked_dicts(std::unordered_map<DictionaryTag, RankedDict> & ranked_dicts) { RankedDicts convert_to_ranked_dicts(
base::flat_map<DictionaryTag, RankedDict>& ranked_dicts) {
RankedDicts build; RankedDicts build;
for (const auto & item : ranked_dicts) { for (const auto & item : ranked_dicts) {
build.insert(item); build.emplace(item.first, &item.second);
} }
return build; return build;
......
...@@ -3,11 +3,10 @@ ...@@ -3,11 +3,10 @@
#include <zxcvbn/frequency_lists_common.hpp> #include <zxcvbn/frequency_lists_common.hpp>
#include <unordered_map>
#include <cstdint> #include <cstdint>
#include "base/strings/string_piece.h" #include "base/strings/string_piece.h"
#include "base/containers/flat_map.h"
namespace zxcvbn { namespace zxcvbn {
...@@ -23,24 +22,13 @@ enum class DictionaryTag { ...@@ -23,24 +22,13 @@ enum class DictionaryTag {
} }
namespace std {
template<>
struct hash<zxcvbn::DictionaryTag> {
std::size_t operator()(const zxcvbn::DictionaryTag & v) const {
return static_cast<std::size_t>(v);
}
};
}
namespace zxcvbn { namespace zxcvbn {
using RankedDicts = std::unordered_map<DictionaryTag, const RankedDict &>; using RankedDicts = base::flat_map<DictionaryTag, const RankedDict*>;
void SetRankedDicts(std::unordered_map<DictionaryTag, RankedDict> dicts); void SetRankedDicts(base::flat_map<DictionaryTag, RankedDict> dicts);
RankedDicts convert_to_ranked_dicts(std::unordered_map<DictionaryTag, RankedDict> & ranked_dicts); RankedDicts convert_to_ranked_dicts(base::flat_map<DictionaryTag, RankedDict> & ranked_dicts);
RankedDicts default_ranked_dicts(); RankedDicts default_ranked_dicts();
} }
......
#ifndef __ZXCVBN__FREQUENCY_LISTS_COMMON_HPP #ifndef __ZXCVBN__FREQUENCY_LISTS_COMMON_HPP
#define __ZXCVBN__FREQUENCY_LISTS_COMMON_HPP #define __ZXCVBN__FREQUENCY_LISTS_COMMON_HPP
#include <cstdint>
#include <string> #include <string>
#include <unordered_map> #include <vector>
#include <utility>
#include <cstdint> #include "base/containers/flat_map.h"
namespace zxcvbn { namespace zxcvbn {
using rank_t = std::size_t; using rank_t = std::size_t;
using RankedDict = std::unordered_map<std::string, rank_t>; using RankedDict = base::flat_map<std::string, rank_t>;
template<class T> template<class T>
RankedDict build_ranked_dict(const T & ordered_list) { RankedDict build_ranked_dict(const T & ordered_list) {
RankedDict result; std::vector<RankedDict::value_type> items;
items.reserve(ordered_list.size());
rank_t idx = 1; // rank starts at 1, not 0 rank_t idx = 1; // rank starts at 1, not 0
for (const auto & word : ordered_list) { for (const auto & word : ordered_list) {
result.insert(std::make_pair(word, idx)); items.emplace_back(word, idx);
idx += 1; idx += 1;
} }
return result; return RankedDict(std::move(items));
} }
} }
......
...@@ -124,8 +124,8 @@ std::vector<Match> omnimatch(const std::string & password, ...@@ -124,8 +124,8 @@ std::vector<Match> omnimatch(const std::string & password,
auto ranked_dictionaries = default_ranked_dicts(); auto ranked_dictionaries = default_ranked_dicts();
auto ranked_dict = build_ranked_dict(ordered_list); auto ranked_dict = build_ranked_dict(ordered_list);
ranked_dictionaries.insert(std::make_pair(DictionaryTag::USER_INPUTS, ranked_dictionaries.insert(
std::cref(ranked_dict))); std::make_pair(DictionaryTag::USER_INPUTS, &ranked_dict));
std::vector<Match> matches; std::vector<Match> matches;
std::function<std::vector<Match>(const std::string&)> matchers[] = { std::function<std::vector<Match>(const std::string&)> matchers[] = {
...@@ -159,7 +159,7 @@ std::vector<Match> dictionary_match(const std::string & password, ...@@ -159,7 +159,7 @@ std::vector<Match> dictionary_match(const std::string & password,
auto password_lower = dict_normalize(password); auto password_lower = dict_normalize(password);
for (const auto & item : ranked_dictionaries) { for (const auto & item : ranked_dictionaries) {
auto dictionary_tag = item.first; auto dictionary_tag = item.first;
auto & ranked_dict = item.second; auto& ranked_dict = *item.second;
for (decltype(len) i = 0, idx = 0; idx < len; util::utf8_decode(password, idx), ++i) { for (decltype(len) i = 0, idx = 0; idx < len; util::utf8_decode(password, idx), ++i) {
for (decltype(len) j = i, jdx = idx; jdx < len; ++j) { for (decltype(len) j = i, jdx = idx; jdx < len; ++j) {
// j is inclusive, but jdx is not so eagerly iterate jdx // j is inclusive, but jdx is not so eagerly iterate jdx
......
diff --git a/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists.cpp b/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists.cpp
index a7adbe95a0d9..4996f7959be4 100644
--- a/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists.cpp
+++ b/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists.cpp
@@ -1,31 +1,32 @@
#include <zxcvbn/frequency_lists.hpp>
-#include <unordered_map>
#include <utility>
+#include "base/containers/flat_map.h"
#include "base/no_destructor.h"
namespace zxcvbn {
namespace {
-std::unordered_map<DictionaryTag, RankedDict>& ranked_dicts() {
- static base::NoDestructor<std::unordered_map<DictionaryTag, RankedDict>>
+base::flat_map<DictionaryTag, RankedDict>& ranked_dicts() {
+ static base::NoDestructor<base::flat_map<DictionaryTag, RankedDict>>
ranked_dicts;
return *ranked_dicts;
}
} // namespace
-void SetRankedDicts(std::unordered_map<DictionaryTag, RankedDict> dicts) {
+void SetRankedDicts(base::flat_map<DictionaryTag, RankedDict> dicts) {
ranked_dicts() = std::move(dicts);
}
-RankedDicts convert_to_ranked_dicts(std::unordered_map<DictionaryTag, RankedDict> & ranked_dicts) {
+RankedDicts convert_to_ranked_dicts(
+ base::flat_map<DictionaryTag, RankedDict>& ranked_dicts) {
RankedDicts build;
for (const auto & item : ranked_dicts) {
- build.insert(item);
+ build.emplace(item.first, &item.second);
}
return build;
diff --git a/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists.hpp b/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists.hpp
index 33da02707152..3757483251e5 100644
--- a/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists.hpp
+++ b/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists.hpp
@@ -3,11 +3,10 @@
#include <zxcvbn/frequency_lists_common.hpp>
-#include <unordered_map>
-
#include <cstdint>
#include "base/strings/string_piece.h"
+#include "base/containers/flat_map.h"
namespace zxcvbn {
@@ -23,24 +22,13 @@ enum class DictionaryTag {
}
-namespace std {
-
-template<>
-struct hash<zxcvbn::DictionaryTag> {
- std::size_t operator()(const zxcvbn::DictionaryTag & v) const {
- return static_cast<std::size_t>(v);
- }
-};
-
-}
-
namespace zxcvbn {
-using RankedDicts = std::unordered_map<DictionaryTag, const RankedDict &>;
+using RankedDicts = base::flat_map<DictionaryTag, const RankedDict*>;
-void SetRankedDicts(std::unordered_map<DictionaryTag, RankedDict> dicts);
+void SetRankedDicts(base::flat_map<DictionaryTag, RankedDict> dicts);
-RankedDicts convert_to_ranked_dicts(std::unordered_map<DictionaryTag, RankedDict> & ranked_dicts);
+RankedDicts convert_to_ranked_dicts(base::flat_map<DictionaryTag, RankedDict> & ranked_dicts);
RankedDicts default_ranked_dicts();
}
diff --git a/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists_common.hpp b/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists_common.hpp
index 40eee250f244..d1c5177fb183 100644
--- a/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists_common.hpp
+++ b/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists_common.hpp
@@ -1,26 +1,27 @@
#ifndef __ZXCVBN__FREQUENCY_LISTS_COMMON_HPP
#define __ZXCVBN__FREQUENCY_LISTS_COMMON_HPP
+#include <cstdint>
#include <string>
-#include <unordered_map>
-#include <utility>
+#include <vector>
-#include <cstdint>
+#include "base/containers/flat_map.h"
namespace zxcvbn {
using rank_t = std::size_t;
-using RankedDict = std::unordered_map<std::string, rank_t>;
+using RankedDict = base::flat_map<std::string, rank_t>;
template<class T>
RankedDict build_ranked_dict(const T & ordered_list) {
- RankedDict result;
+ std::vector<RankedDict::value_type> items;
+ items.reserve(ordered_list.size());
rank_t idx = 1; // rank starts at 1, not 0
for (const auto & word : ordered_list) {
- result.insert(std::make_pair(word, idx));
+ items.emplace_back(word, idx);
idx += 1;
}
- return result;
+ return RankedDict(std::move(items));
}
}
diff --git a/third_party/zxcvbn-cpp/native-src/zxcvbn/matching.cpp b/third_party/zxcvbn-cpp/native-src/zxcvbn/matching.cpp
index 6d43cd2dc20b..8f4e6d2f0e00 100644
--- a/third_party/zxcvbn-cpp/native-src/zxcvbn/matching.cpp
+++ b/third_party/zxcvbn-cpp/native-src/zxcvbn/matching.cpp
@@ -124,8 +124,8 @@ std::vector<Match> omnimatch(const std::string & password,
auto ranked_dictionaries = default_ranked_dicts();
auto ranked_dict = build_ranked_dict(ordered_list);
- ranked_dictionaries.insert(std::make_pair(DictionaryTag::USER_INPUTS,
- std::cref(ranked_dict)));
+ ranked_dictionaries.insert(
+ std::make_pair(DictionaryTag::USER_INPUTS, &ranked_dict));
std::vector<Match> matches;
std::function<std::vector<Match>(const std::string&)> matchers[] = {
@@ -159,7 +159,7 @@ std::vector<Match> dictionary_match(const std::string & password,
auto password_lower = dict_normalize(password);
for (const auto & item : ranked_dictionaries) {
auto dictionary_tag = item.first;
- auto & ranked_dict = item.second;
+ auto& ranked_dict = *item.second;
for (decltype(len) i = 0, idx = 0; idx < len; util::utf8_decode(password, idx), ++i) {
for (decltype(len) j = i, jdx = idx; jdx < len; ++j) {
// j is inclusive, but jdx is not so eagerly iterate jdx
...@@ -164,7 +164,7 @@ bool operator==(const Match& lhs, const ExpectedDateMatch& rhs) { ...@@ -164,7 +164,7 @@ bool operator==(const Match& lhs, const ExpectedDateMatch& rhs) {
TEST(ZxcvbnTest, DictionaryMatching) { TEST(ZxcvbnTest, DictionaryMatching) {
auto dict_1 = static_cast<DictionaryTag>(0); auto dict_1 = static_cast<DictionaryTag>(0);
auto dict_2 = static_cast<DictionaryTag>(1); auto dict_2 = static_cast<DictionaryTag>(1);
std::unordered_map<DictionaryTag, RankedDict> test_dicts = { base::flat_map<DictionaryTag, RankedDict> test_dicts = {
{dict_1, {dict_1,
{ {
{"motherboard", 1}, {"motherboard", 1},
...@@ -344,7 +344,7 @@ TEST(ZxcvbnTest, DictionaryMatching) { ...@@ -344,7 +344,7 @@ TEST(ZxcvbnTest, DictionaryMatching) {
TEST(ZxcvbnTest, ReverseDictionaryMatching) { TEST(ZxcvbnTest, ReverseDictionaryMatching) {
auto dict_1 = static_cast<DictionaryTag>(0); auto dict_1 = static_cast<DictionaryTag>(0);
std::unordered_map<DictionaryTag, RankedDict> test_dicts = { base::flat_map<DictionaryTag, RankedDict> test_dicts = {
{dict_1, {dict_1,
{ {
{"123", 1}, {"123", 1},
...@@ -430,7 +430,7 @@ TEST(ZxcvbnTest, L33tMatching) { ...@@ -430,7 +430,7 @@ TEST(ZxcvbnTest, L33tMatching) {
{ {
auto words = static_cast<DictionaryTag>(0); auto words = static_cast<DictionaryTag>(0);
auto words2 = static_cast<DictionaryTag>(1); auto words2 = static_cast<DictionaryTag>(1);
std::unordered_map<DictionaryTag, RankedDict> dicts = { base::flat_map<DictionaryTag, RankedDict> dicts = {
{words, {words,
{ {
{"aac", 1}, {"aac", 1},
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment