Skip to content

Commit

Permalink
Don't run 200-entry quadratic-time insertion sorts at compile time
Browse files Browse the repository at this point in the history
MakeFixedFlatMap runs a compile-time insertion sort. There were a couple
of very large ones in print_media_l10n.cc. When base::StringPiece is
switched to std::string_view, they blow through Clang's constexpr
evaluation limit. Probably they were already near the edge, and
std::string_view has more steps over base::StringPiece, by constexpr's
counting, and that pushed this over the limit.

Add a variant of MakeFixedFlatMap where the input is required to be (and
checked to be) pre-sorted.

Bug: 691162
Change-Id: I913532cc8fc0d7beadff07bf7ba17aba5420530a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4296340
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1111651}
  • Loading branch information
davidben authored and ReyBan82 committed Feb 27, 2024
1 parent 3deaa6f commit 1019913
Show file tree
Hide file tree
Showing 2 changed files with 223 additions and 205 deletions.
31 changes: 25 additions & 6 deletions base/containers/fixed_flat_map.h
Expand Up @@ -89,18 +89,19 @@ using fixed_flat_map = base::
flat_map<Key, Mapped, Compare, std::array<std::pair<const Key, Mapped>, N>>;

// Utility function to simplify constructing a fixed_flat_map from a fixed list
// of keys and values. Requires that the passed in `data` contains unique keys.
// of keys and values. Requires that the passed in `data` contains unique keys
// and be sorted by key. See `MakeFixedFlatMap` for a variant that sorts the
// input automatically.
//
// Example usage:
// constexpr auto kMap = base::MakeFixedFlatMap<base::StringPiece, int>(
// {{"foo", 1}, {"bar", 2}, {"baz", 3}});
// constexpr auto kMap = base::MakeFixedFlatMapSorted<base::StringPiece, int>(
// {{"bar", 2}, {"baz", 3}, {"foo", 1}});
template <class Key, class Mapped, size_t N, class Compare = std::less<>>
constexpr fixed_flat_map<Key, Mapped, N, Compare> MakeFixedFlatMap(
std::pair<Key, Mapped>(&&data)[N],
constexpr fixed_flat_map<Key, Mapped, N, Compare> MakeFixedFlatMapSorted(
std::pair<Key, Mapped> (&&data)[N],
const Compare& comp = Compare()) {
using FixedFlatMap = fixed_flat_map<Key, Mapped, N, Compare>;
typename FixedFlatMap::value_compare value_comp{comp};
internal::InsertionSort(data, data + N, value_comp);
CHECK(internal::is_sorted_and_unique(data, value_comp));
// Specify the value_type explicitly to ensure that the returned array has
// immutable keys.
Expand All @@ -109,6 +110,24 @@ constexpr fixed_flat_map<Key, Mapped, N, Compare> MakeFixedFlatMap(
comp);
}

// Utility function to simplify constructing a fixed_flat_map from a fixed list
// of keys and values. Requires that the passed in `data` contains unique keys.
// This function does a quadratic insertion sort at compile-time, so if your set
// is large, prefer `MakeFixedFlatMapSorted`.
//
// Example usage:
// constexpr auto kMap = base::MakeFixedFlatMap<base::StringPiece, int>(
// {{"foo", 1}, {"bar", 2}, {"baz", 3}});
template <class Key, class Mapped, size_t N, class Compare = std::less<>>
constexpr fixed_flat_map<Key, Mapped, N, Compare> MakeFixedFlatMap(
std::pair<Key, Mapped> (&&data)[N],
const Compare& comp = Compare()) {
using FixedFlatMap = fixed_flat_map<Key, Mapped, N, Compare>;
typename FixedFlatMap::value_compare value_comp{comp};
internal::InsertionSort(data, data + N, value_comp);
return MakeFixedFlatMapSorted(std::move(data), comp);
}

} // namespace base

#endif // BASE_CONTAINERS_FIXED_FLAT_MAP_H_

0 comments on commit 1019913

Please sign in to comment.