Replace hashtable ForEach callback with range-based iteration (#7806)

Replaces the callback-based `ForEach` methods on `RawHashtable`, `Map`,
and
`Set` with a range object supporting range-for loops, structured
bindings, and
the standard range concepts.

- Adds `.entries()` on `Map`, `Set`, and `RawHashtable`, returning a
range that
  models `std::ranges::forward_range` and `std::ranges::common_range`.
  Obtaining one is an explicit call rather than `begin()`/`end()` on the
container, as scanning a whole table is costly and shouldn't be hidden.
- Iterating a `Map` yields a `std::pair` of key and value references,
which
fits in two registers and is returned without being materialized in
memory.
- `Map::Range` and `Set::Range` are aliases of the raw hashtable's range
rather
than wrappers around it. The raw iterator produces the user-facing
reference
itself -- a `KeyT&` for a set, a pair of references for a map -- picked
by
`StorageEntry`, which is already specialized on whether there is a value
  type. That leaves one iterator to reason about instead of three.
- Deletes the rvalue `.entries()` overloads on the owning containers, as
a
  range built from a temporary table would dangle. Views don't own their
  storage, so the operation remains available on them.
- In release builds, the walk over the groups is a single induction
variable: a
  negative byte offset counting up to zero, anchored at the ends of the
metadata and entry arrays. Both arrays are then reached by indexed
addressing
off a base that stays put, and the entry pointer is formed only once a
group
  with a present entry has been found.
- In debug builds, the range hashes the table's metadata when it is
built and
re-checks that hash when it is destroyed, catching mutation of the table
while a range is live. It also picks a random starting group and a
random odd
group stride, which varies the traversal order between ranges while
still
visiting every group exactly once. That entropy is drawn when the range
is
built rather than in `begin()`, so `begin()` stays a pure function of
the
  range and the multi-pass guarantee holds.
- Removes `ForEachEntry` and all of its callers.

Measured against the iteration benchmark added in its own commit, a
traversal is at or ahead of what the callback compiled to across nearly
the
whole size range. The largest tables spend 3-5% fewer cycles, small
`Set`s as
much as 24% fewer, and instruction counts stay within about 1%. What
remains
behind is a handful of mid-sized `Map`s by up to 1%, and `Set` at 65536,
which
sits at exactly half its load factor, by 2%.

Both revisions were built with `-c opt --copt=-march=x86-64-v3` and
compared
with:

```
./scripts/bench_runner.py --exp_benchmark=... --base_benchmark=... \
    --benchmark_args=--benchmark_perf_counters=INSTRUCTIONS,CYCLES \
    --benchmark_args='--benchmark_filter=(Set|Map)Iterate<(Set|Map)<' \
    --extra_metrics_filter='(INSTRUCTIONS|CYCLES)'
```

Trimmed below to the primary integer configurations and to the two
counters;
the pointer- and string-keyed configurations follow the same pattern.

```
 Benchmark                             ┃           CYCLES            ┃        INSTRUCTIONS
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 BM_MapIterate<Map<int, int>>/1....... │ 👍  -6.032%      p=1.14e-05 │      ??          p=0.752
                             baseline: │     12.06      ±   1.520%   │     64         ±   3.125%
                           experiment: │     11.33      ±   2.765%   │     65.5       ±   3.817%
                                       │                             │
 BM_MapIterate<Map<int, int>>/2....... │      ??          p=0.155    │      ??          p=0.343
                             baseline: │      7.587     ±   1.285%   │     41         ±   0.000%
                           experiment: │      7.652     ±   0.865%   │     41         ±   2.439%
                                       │                             │
 BM_MapIterate<Map<int, int>>/3....... │      ??          p=0.343    │ 👍  -1.020%      p=0.0039
                             baseline: │      6.663     ±   4.260%   │     32.67      ±   2.041%
                           experiment: │      6.368     ±  12.224%   │     32.33      ±   2.062%
                                       │                             │
 BM_MapIterate<Map<int, int>>/4....... │      ??          p=0.343    │ 👍  -1.786%      p=0.0297
                             baseline: │      6.091     ±  15.470%   │     28         ±   3.571%
                           experiment: │      5.957     ±   8.932%   │     27.5       ±   3.636%
                                       │                             │
 BM_MapIterate<Map<int, int>>/8....... │      ??          p=0.323    │ 👍  -1.220%      p=0.000148
                             baseline: │      4.845     ±   0.800%   │     20.5       ±   0.000%
                           experiment: │      4.814     ±   3.585%   │     20.25      ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/16...... │ 👍  -2.195%      p=0.00908  │ 👍   0.769%      p=6.58e-06
                             baseline: │      4.312     ±   0.187%   │     16.25      ±   0.000%
                           experiment: │      4.218     ±   2.368%   │     16.13      ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/32...... │      ??          p=0.236    │ 👍   0.442%      p=9.53e-06
                             baseline: │      4.051     ±   1.084%   │     14.13      ±   0.000%
                           experiment: │      4.063     ±   0.737%   │     14.06      ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/64...... │      ??          p=0.693    │ 👎   0.227%      p=4.52e-06
                             baseline: │      4.021     ±   0.239%   │     13.75      ±   0.000%
                           experiment: │      4.019     ±   0.417%   │     13.78      ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/256..... │ 👍   0.360%      p=0.00119  │ 👎   0.754%      p=1.37e-05
                             baseline: │      3.996     ±   0.173%   │     13.47      ±   0.000%
                           experiment: │      3.982     ±   0.272%   │     13.57      ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/4096.... │ 👍   0.581%      p=1.96e-05 │ 👎   0.923%      p=1.96e-05
                             baseline: │      4.005     ±   0.816%   │     13.38      ±   0.000%
                           experiment: │      3.981     ±   0.192%   │     13.5       ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/65536... │ 👍  -4.957%      p=1.14e-05 │ 👎   0.934%      p=1.14e-05
                             baseline: │      5.307     ±   0.501%   │     13.38      ±   0.000%
                           experiment: │      5.044     ±   1.746%   │     13.5       ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/1048576. │ 👍  -3.947%      p=9.09e-05 │ 👎   0.935%      p=3.3e-05
                             baseline: │      6.074     ±   0.807%   │     13.38      ±   0.000%
                           experiment: │      5.834     ±   2.159%   │     13.5       ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/16777216 │      ??          p=0.155    │ 👎   0.935%      p=2.11e-05
                             baseline: │      5.082     ±   3.650%   │     13.38      ±   0.000%
                           experiment: │      5.012     ±   1.316%   │     13.5       ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/56...... │ 👎   0.825%      p=0.0268   │ 👍   0.270%      p=1.14e-05
                             baseline: │      3.918     ±   0.501%   │     13.21      ±   0.000%
                           experiment: │      3.951     ±   0.342%   │     13.18      ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/224..... │ 👎   0.788%      p=0.000504 │ 👎   0.346%      p=1.64e-05
                             baseline: │      3.895     ±   0.111%   │     12.89      ±   0.000%
                           experiment: │      3.926     ±   0.285%   │     12.94      ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/3584.... │ 👎   1.028%      p=0.000148 │ 👎   0.545%      p=1.14e-05
                             baseline: │      3.913     ±   0.427%   │     12.79      ±   0.000%
                           experiment: │      3.954     ±   0.325%   │     12.86      ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/57344... │      ??          p=0.236    │ 👎   0.558%      p=2.55e-06
                             baseline: │      4.574     ±   0.721%   │     12.79      ±   0.000%
                           experiment: │      4.51      ±   3.709%   │     12.86      ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/917504.. │ 👍  -3.826%      p=6.58e-06 │ 👎   0.559%      p=2.33e-05
                             baseline: │      5.221     ±   0.507%   │     12.79      ±   0.000%
                           experiment: │      5.021     ±   0.556%   │     12.86      ±   0.000%
                                       │                             │
 BM_MapIterate<Map<int, int>>/14680064 │ 👍  -3.839%      p=1.37e-05 │ 👎   0.559%      p=3.31e-05
                             baseline: │      5.129     ±   1.194%   │     12.79      ±   0.000%
                           experiment: │      4.932     ±   1.475%   │     12.86      ±   0.000%
                                       │                             │

 Benchmark                        ┃           CYCLES            ┃        INSTRUCTIONS
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 BM_SetIterate<Set<int>>/1....... │ 👍  -3.104%      p=0.000583 │      ??          p=0.206
                        baseline: │     11.2       ±   6.323%   │     60         ±   3.333%
                      experiment: │     10.85      ±   5.820%   │     61         ±   3.279%
                                  │                             │
 BM_SetIterate<Set<int>>/2....... │ 👍  -7.037%      p=0.0362   │      ??          p=0.155
                        baseline: │      7.086     ±  16.857%   │     37         ±   0.000%
                      experiment: │      6.587     ±   0.479%   │     36         ±   4.167%
                                  │                             │
 BM_SetIterate<Set<int>>/3....... │ 👎   1.400%      p=2.34e-05 │ 👍  -1.163%      p=0.00136
                        baseline: │      5.363     ±   0.463%   │     28.67      ±   2.326%
                      experiment: │      5.438     ±  32.763%   │     28.33      ±   1.176%
                                  │                             │
 BM_SetIterate<Set<int>>/4....... │      ??          p=0.968    │      ??          p=0.286
                        baseline: │      4.642     ±  32.751%   │     23.5       ±   2.128%
                      experiment: │      4.658     ±  38.416%   │     23.63      ±   3.704%
                                  │                             │
 BM_SetIterate<Set<int>>/8....... │ 👍 -23.823%      p=3.74e-06 │ 👍  -1.515%      p=5.52e-05
                        baseline: │      4.701     ±   6.589%   │     16.5       ±   0.000%
                      experiment: │      3.581     ±   7.790%   │     16.25      ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/16...... │ 👍  -4.502%      p=1.37e-05 │ 👍  -1.020%      p=3.31e-05
                        baseline: │      3.124     ±   0.585%   │     12.25      ±   0.000%
                      experiment: │      2.983     ±   0.625%   │     12.13      ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/32...... │ 👍  -4.032%      p=5.46e-06 │ 👍   0.617%      p=1.96e-05
                        baseline: │      2.957     ±   0.260%   │     10.13      ±   0.000%
                      experiment: │      2.838     ±   0.434%   │     10.06      ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/64...... │ 👍  -5.054%      p=4.52e-06 │ 👎   0.321%      p=1.37e-05
                        baseline: │      2.937     ±   0.301%   │      9.75      ±   0.000%
                      experiment: │      2.788     ±   1.143%   │      9.781     ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/256..... │ 👍  -5.325%      p=1.14e-05 │ 👎   1.073%      p=6.58e-06
                        baseline: │      2.916     ±   0.220%   │      9.469     ±   0.000%
                      experiment: │      2.761     ±   0.142%   │      9.57      ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/4096.... │ 👍  -4.865%      p=4.52e-06 │ 👎   1.317%      p=2.34e-05
                        baseline: │      2.921     ±   0.194%   │      9.381     ±   0.000%
                      experiment: │      2.779     ±   0.224%   │      9.504     ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/65536... │ 👎   1.961%      p=3.93e-05 │ 👎   1.332%      p=1.49e-05
                        baseline: │      4.015     ±   0.482%   │      9.375     ±   0.000%
                      experiment: │      4.094     ±   0.613%   │      9.5       ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/1048576. │ 👍  -4.843%      p=1.14e-05 │ 👎   1.333%      p=5.38e-06
                        baseline: │      5.239     ±   0.144%   │      9.375     ±   0.000%
                      experiment: │      4.986     ±   0.139%   │      9.5       ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/16777216 │ 👍   0.840%      p=0.0362   │ 👎   1.333%      p=2.52e-06
                        baseline: │      3.719     ±   1.420%   │      9.375     ±   0.000%
                      experiment: │      3.688     ±   1.308%   │      9.5       ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/56...... │ 👍  -2.857%      p=9.53e-06 │ 👍   0.388%      p=3.31e-05
                        baseline: │      2.942     ±   0.439%   │      9.214     ±   0.000%
                      experiment: │      2.858     ±   0.619%   │      9.179     ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/224..... │ 👍  -2.161%      p=2.34e-05 │ 👎   0.502%      p=4.52e-06
                        baseline: │      2.888     ±   0.347%   │      8.893     ±   0.000%
                      experiment: │      2.826     ±   0.450%   │      8.938     ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/3584.... │ 👍  -1.750%      p=6.58e-06 │ 👎   0.793%      p=2.34e-05
                        baseline: │      2.89      ±   0.261%   │      8.792     ±   0.000%
                      experiment: │      2.84      ±   0.411%   │      8.862     ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/57344... │      ??          p=0.502    │ 👎   0.812%      p=2.78e-05
                        baseline: │      3.684     ±   4.246%   │      8.786     ±   0.000%
                      experiment: │      3.644     ±   4.431%   │      8.857     ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/917504.. │ 👍  -2.629%      p=0.000148 │ 👎   0.813%      p=3.08e-06
                        baseline: │      4.372     ±   0.693%   │      8.786     ±   0.000%
                      experiment: │      4.257     ±   0.210%   │      8.857     ±   0.000%
                                  │                             │
 BM_SetIterate<Set<int>>/14680064 │ 👍  -2.927%      p=0.0219   │ 👎   0.813%      p=3.03e-06
                        baseline: │      4.154     ±   3.286%   │      8.786     ±   0.000%
                      experiment: │      4.032     ±   3.198%   │      8.857     ±   0.000%
                                  │                             │
```

Assisted-by: Antigravity with Opus
This commit is contained in:
Chandler Carruth
2026-09-18 20:19:46 +00:00
committed by GitHub
parent 681b3b10c4
commit d037848a96
12 changed files with 909 additions and 143 deletions
+21 -18
View File
@@ -69,6 +69,14 @@ class MapView
using KeyContextT = ImplT::KeyContextT;
using MetricsT = ImplT::MetricsT;
// A key and its value, as a pair of references. This is what iterating the
// map produces; there is no object in the table combining the two.
using Entry = ImplT::EntryRefT;
// A range over the key-value entries of the map. Bound to the lifetime of
// the viewed map, and invalidated by mutating it.
using Range = ImplT::EntryRange;
// This type represents the result of lookup operations. It encodes whether
// the lookup was a success as well as accessors for the key and value.
class LookupKVResult {
@@ -111,10 +119,8 @@ class MapView
auto operator[](LookupKeyT lookup_key) const -> ValueT*
requires(std::default_initializable<KeyContextT>);
// Run the provided callback for every key and value in the map.
template <typename CallbackT>
auto ForEach(CallbackT callback) -> void
requires(std::invocable<CallbackT, KeyT&, ValueT&>);
// Returns a range for iterating over all key-value entries in the map.
auto entries() const -> Range;
// This routine is relatively inefficient and only intended for use in
// benchmarking or logging of performance anomalies. The specific metrics
@@ -169,6 +175,8 @@ class MapBase : protected RawHashtable::BaseImpl<InputKeyT, InputValueT,
using ViewT = MapView<KeyT, ValueT, KeyContextT>;
using LookupKVResult = ViewT::LookupKVResult;
using MetricsT = ImplT::MetricsT;
using Entry = ViewT::Entry;
using Range = ViewT::Range;
// The result type for insertion operations both indicates whether an insert
// was needed (as opposed to finding an existing element), and provides access
@@ -228,12 +236,12 @@ class MapBase : protected RawHashtable::BaseImpl<InputKeyT, InputValueT,
}
// Convenience forwarder to the view type.
template <typename CallbackT>
auto ForEach(CallbackT callback) const -> void
requires(std::invocable<CallbackT, KeyT&, ValueT&>)
{
return ViewT(*this).ForEach(callback);
}
auto entries() const& -> Range { return ViewT(*this).entries(); }
// Deleted on rvalues: the range refers to storage owned by this table, so a
// range built from a temporary map would dangle. Both qualifiers are needed
// as `&&` alone would leave a const rvalue binding to the `const&` overload.
auto entries() && = delete;
auto entries() const&& = delete;
// Convenience forwarder to the view type.
auto ComputeMetrics(KeyContextT key_context = KeyContextT()) const
@@ -424,14 +432,9 @@ auto MapView<InputKeyT, InputValueT, InputKeyContextT>::operator[](
}
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
template <typename CallbackT>
auto MapView<InputKeyT, InputValueT, InputKeyContextT>::ForEach(
CallbackT callback) -> void
requires(std::invocable<CallbackT, KeyT&, ValueT&>)
{
this->ForEachEntry(
[callback](EntryT& entry) { callback(entry.key(), entry.value()); },
[](auto...) {});
auto MapView<InputKeyT, InputValueT, InputKeyContextT>::entries() const
-> Range {
return this->ImplT::EntriesImpl();
}
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
+58
View File
@@ -93,6 +93,17 @@ struct MapWrapperImpl {
}
auto BenchErase(KeyT k) -> bool { return m.erase(k) != 0; }
// Visits every entry in the map, calling `cb` with the key and value of each
// one. Each map type is expected to traverse using whatever API it provides
// for this, so that the benchmark measures iterating the map rather than any
// specific iteration API.
template <typename CallbackT>
auto BenchIterate(CallbackT cb) -> void {
for (const auto& entry : m) {
cb(entry.first, entry.second);
}
}
};
// Explicit (partial) specialization for the Carbon map type that uses its
@@ -126,6 +137,13 @@ struct MapWrapperImpl<Map<KT, VT, MinSmallSize>> {
}
auto BenchErase(KeyT k) -> bool { return m.Erase(k); }
template <typename CallbackT>
auto BenchIterate(CallbackT cb) -> void {
for (auto [k, v] : m.entries()) {
cb(k, v);
}
}
};
// Provide a way to override the Carbon Map specific benchmark runs with another
@@ -516,5 +534,45 @@ static void BM_MapInsertSeq(benchmark::State& state) {
}
MAP_BENCHMARK_ONE_OP(BM_MapInsertSeq, SizeArgs);
// Benchmark visiting every entry in a map.
//
// Unlike the lookup benchmarks, this walks the table's storage from end to end
// rather than probing it, so it is largely a measure of how densely entries are
// packed and how cheaply empty slots can be skipped. There is no dependency
// between the entries visited, and so this is a throughput measurement.
//
// Each batch is a single complete traversal of the map, with the batch size set
// to the number of entries so that the reported time is the per-entry cost.
template <typename MapT>
static void BM_MapIterate(benchmark::State& state) {
using MapWrapperT = MapWrapper<MapT>;
using KT = typename MapWrapperT::KeyT;
using VT = typename MapWrapperT::ValueT;
MapWrapperT m;
auto [keys, _] = GetKeysAndMissKeys<KT>(state.range(0));
for (auto k : keys) {
bool inserted = m.BenchInsert(k, MakeValue<VT>());
CARBON_DCHECK(inserted, "Must be a successful insert!");
}
while (state.KeepRunningBatch(keys.size())) {
ssize_t sum = 0;
m.BenchIterate([&sum](const KT& k, const VT& v) {
// Consume both the key and the value so that neither the traversal nor
// the loads out of the entries can be optimized away.
sum += ValueToBool(k) + ValueToBool(v);
});
benchmark::DoNotOptimize(sum);
}
// The time is already per-entry, so an iteration-invariant rate of one gives
// the throughput of entries visited.
state.counters["KeyRate"] =
benchmark::Counter(1, benchmark::Counter::kIsIterationInvariantRate);
ReportMetrics(m, state);
}
MAP_BENCHMARK_ONE_OP(BM_MapIterate, SizeArgs);
} // namespace
} // namespace Carbon
+113 -2
View File
@@ -7,7 +7,10 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <concepts>
#include <initializer_list>
#include <iterator>
#include <ranges>
#include <type_traits>
#include <utility>
#include <vector>
@@ -37,6 +40,7 @@ using RawHashtable::MoveOnlyTestData;
using RawHashtable::TestData;
using RawHashtable::TestKeyContext;
using ::testing::Pair;
using ::testing::UnorderedElementsAre;
using ::testing::UnorderedElementsAreArray;
template <typename MapT, typename MatcherRangeT>
@@ -47,9 +51,9 @@ auto ExpectMapElementsAre(MapT&& m, MatcherRangeT element_matchers) -> void {
std::vector<
std::pair<std::reference_wrapper<KeyT>, std::reference_wrapper<ValueT>>>
map_entries;
m.ForEach([&map_entries](KeyT& k, ValueT& v) {
for (auto [k, v] : m.entries()) {
map_entries.push_back({std::ref(k), std::ref(v)});
});
}
// Use the GoogleMock unordered container matcher to validate and show errors
// on wrong elements.
@@ -865,5 +869,112 @@ TEST(MapContextTest, Basic) {
m, MakeKeyValues([](int k) { return k * 100 + 1; }, llvm::seq(1, 512)));
}
TYPED_TEST(MapTest, Range) {
using MapT = TypeParam;
using Range = decltype(std::declval<const MapT&>().entries());
using Iter = typename Range::Iterator;
static_assert(std::forward_iterator<Iter>);
static_assert(std::same_as<decltype(std::declval<Range>().begin()), Iter>);
static_assert(std::same_as<decltype(std::declval<Range>().end()), Iter>);
static_assert(std::ranges::forward_range<Range>);
static_assert(std::ranges::common_range<Range>);
MapT m;
EXPECT_EQ(m.entries().begin(), m.entries().end());
for (auto [k, v] : m.entries()) {
static_cast<void>(k);
static_cast<void>(v);
FAIL() << "Empty map range should have no elements";
}
for (int i = 1; i <= 5; ++i) {
m.Insert(i, i * 10);
}
int count = 0;
for (const auto& [k, v] : m.entries()) {
EXPECT_EQ(v, m.Lookup(k).value());
++count;
}
EXPECT_EQ(count, 5);
EXPECT_THAT(m.entries(),
UnorderedElementsAre(Pair(1, 10), Pair(2, 20), Pair(3, 30),
Pair(4, 40), Pair(5, 50)));
using KeyT = typename MapT::KeyT;
using ValueT = typename MapT::ValueT;
using KeyContextT = typename MapT::KeyContextT;
MapView<const KeyT, const ValueT, KeyContextT> cv = m;
int cv_count = 0;
for (auto [k, v] : cv.entries()) {
static_assert(std::is_const_v<std::remove_reference_t<decltype(k)>>);
static_assert(std::is_const_v<std::remove_reference_t<decltype(v)>>);
EXPECT_EQ(v, m.Lookup(k).value());
++cv_count;
}
EXPECT_EQ(cv_count, 5);
EXPECT_THAT(cv.entries(),
UnorderedElementsAre(Pair(1, 10), Pair(2, 20), Pair(3, 30),
Pair(4, 40), Pair(5, 50)));
for (auto [k, v] : m.entries()) {
if constexpr (requires { v.value; }) {
v.value = 99;
} else {
v = 99;
}
}
for (const auto& [k, v] : m.entries()) {
if constexpr (requires { v.value; }) {
EXPECT_EQ(v.value, 99);
} else {
EXPECT_EQ(v, 99);
}
}
EXPECT_THAT(m.entries(),
UnorderedElementsAre(Pair(1, 99), Pair(2, 99), Pair(3, 99),
Pair(4, 99), Pair(5, 99)));
auto r = m.entries();
int iter_count = 0;
for (auto it = r.begin(); it != r.end(); ++it) {
EXPECT_EQ(it->second, m.Lookup(it->first).value());
EXPECT_EQ((*it).second, m.Lookup((*it).first).value());
++iter_count;
}
EXPECT_EQ(iter_count, 5);
auto it = r.begin();
auto prev = it++;
EXPECT_NE(it, prev);
}
TYPED_TEST(MoveOnlyMapTest, Range) {
TypeParam m;
m.Insert(1, 10);
m.Insert(2, 20);
int count = 0;
for (auto [k, v] : m.entries()) {
EXPECT_EQ(v.value, k.value * 10);
++count;
}
EXPECT_EQ(count, 2);
}
#ifndef NDEBUG
TEST(MapDeathTest, MutateDuringIterationFails) {
EXPECT_DEATH(([] {
Map<int, int> m;
m.Insert(1, 10);
auto range = m.entries();
m.Insert(2, 20);
}()),
"Hashtable mutated during iteration");
}
#endif
} // namespace
} // namespace Carbon::Testing
+5
View File
@@ -10,4 +10,9 @@ namespace Carbon::RawHashtable {
volatile std::byte global_addr_seed{1};
#ifndef NDEBUG
std::atomic<HashCode> entropy_hash =
Carbon::HashValue(reinterpret_cast<uint64_t>(&global_addr_seed));
#endif
} // namespace Carbon::RawHashtable
+423 -79
View File
@@ -6,6 +6,7 @@
#define CARBON_COMMON_RAW_HASHTABLE_H_
#include <algorithm>
#include <atomic>
#include <concepts>
#include <cstddef>
#include <cstring>
@@ -18,6 +19,7 @@
#include "common/concepts.h"
#include "common/hashing.h"
#include "common/raw_hashtable_metadata_group.h"
#include "llvm/ADT/iterator.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/MathExtras.h"
@@ -122,10 +124,15 @@
// null. Since it doesn't track the exact number of filled entries in a table,
// it doesn't support a container-style `size` API.
//
// - There is no direct iterator support because of the complexity of embedding
// the group-based metadata scanning into an iterator model. Instead, there is
// just a for-each method that is passed a lambda to observe all entries. The
// order of this observation is also not guaranteed.
// - Iteration is provided by a range object rather than by iterators hanging
// directly off the table, because the debug-only checks for mutation during
// iteration need state that outlives a single iterator: see `EntryRange`
// below. Obtaining one is an explicit call (`entries()`), as scanning an
// entire table is a costly operation that shouldn't be hidden behind a bare
// `begin()`/`end()` pair.
//
// The order of iteration is not guaranteed, and debug builds actively vary it
// between ranges to keep callers from depending on it.
namespace Carbon::RawHashtable {
// Which prefetch strategies to enable can be controlled via macros to enable
@@ -152,7 +159,7 @@ inline constexpr ssize_t MinAllocatedSize = std::max<ssize_t>(64, MaxGroupSize);
// An entry in the hashtable storage of a `KeyT` and `ValueT` object.
//
// Allows manual construction, destruction, and access to these values so we can
// create arrays af the entries prior to populating them with actual keys and
// create arrays of the entries prior to populating them with actual keys and
// values.
template <typename KeyT, typename ValueT>
struct StorageEntry {
@@ -168,6 +175,20 @@ struct StorageEntry {
IsTriviallyRelocatable || (std::is_copy_constructible_v<KeyT> &&
std::is_copy_constructible_v<ValueT>);
// How iteration refers to an entry, and the iterator traits that follow.
//
// The key and value are stored side by side with nothing combining them, so
// a reference to an entry is a pair of references built on demand. That pair
// is a *proxy* reference: C++20 forward iterators permit one, but C++17
// algorithms may assume a forward iterator's reference is a real lvalue, so
// the C++17 category is `input`.
using RefT = std::pair<KeyT&, ValueT&>;
using IterValueT = RefT;
using IterPointerT = const RefT*;
using IterCategoryT = std::input_iterator_tag;
auto ref() -> RefT { return RefT(key(), value()); }
auto key() const -> const KeyT& {
// Ensure we don't need more alignment than available. Inside a method body
// to apply to the complete type.
@@ -194,11 +215,21 @@ struct StorageEntry {
// construction. As a consequence, this struct only provides the storage and
// we have to manually manage the construction, move, and destruction of the
// objects.
//
// Destroys the key and value behind an entry reference. Iteration hands back
// `RefT` rather than the entry, so this is how a walked entry is destroyed.
static auto DestroyRef(RefT ref) -> void {
ref.first.~KeyT();
ref.second.~ValueT();
}
// Destroys the key and value of this entry. The common case is destroying an
// entry found in the table's storage, where there is no reference to hand to
// `DestroyRef`.
auto Destroy() -> void {
static_assert(!IsTriviallyDestructible,
"Should never instantiate when trivial!");
key().~KeyT();
value().~ValueT();
DestroyRef(ref());
}
auto CopyFrom(const StorageEntry& entry) -> void {
@@ -241,6 +272,15 @@ struct StorageEntry<KeyT, void> {
static constexpr bool IsCopyable =
IsTriviallyRelocatable || std::is_copy_constructible_v<KeyT>;
// As above, but a set's entry is nothing but its key, so a reference to an
// entry is a true lvalue reference and the iterator is a plain forward one.
using RefT = KeyT&;
using IterValueT = std::remove_cv_t<KeyT>;
using IterPointerT = KeyT*;
using IterCategoryT = std::forward_iterator_tag;
auto ref() -> RefT { return key(); }
auto key() const -> const KeyT& {
// Ensure we don't need more alignment than available.
static_assert(
@@ -254,10 +294,12 @@ struct StorageEntry<KeyT, void> {
return const_cast<KeyT&>(const_cast<const StorageEntry*>(this)->key());
}
static auto DestroyRef(RefT ref) -> void { ref.~KeyT(); }
auto Destroy() -> void {
static_assert(!IsTriviallyDestructible,
"Should never instantiate when trivial!");
key().~KeyT();
DestroyRef(ref());
}
auto CopyFrom(const StorageEntry& entry) -> void
@@ -360,6 +402,13 @@ class ViewImpl {
using EntryT = StorageEntry<KeyT, ValueT>;
using MetricsT = Metrics;
// What iterating over the table's entries produces: a `KeyT&` for a set, and
// a `std::pair<KeyT&, ValueT&>` for a map. See `StorageEntry`.
using EntryRefT = EntryT::RefT;
// The range type produced by `EntriesImpl`.
class EntryRange;
friend class BaseImpl<KeyT, ValueT, KeyContextT>;
template <typename InputBaseT, ssize_t SmallSize>
friend class TableImpl;
@@ -385,13 +434,11 @@ class ViewImpl {
auto LookupEntry(LookupKeyT lookup_key, KeyContextT key_context) const
-> EntryT*;
// Calls `entry_callback` for each entry in the hashtable. All the entries
// within a specific group are visited first, and then `group_callback` is
// called on the group itself. The `group_callback` is typically only used by
// the internals of the hashtable.
template <typename EntryCallbackT, typename GroupCallbackT>
auto ForEachEntry(EntryCallbackT entry_callback,
GroupCallbackT group_callback) const -> void;
// Returns a range for iterating over all entries in the hashtable.
//
// The returned range copies this view, so it remains valid for as long as the
// underlying table does, independent of this view's lifetime.
auto EntriesImpl() const -> EntryRange;
// Returns a collection of informative metrics on the the current state of the
// table, useful for performance analysis. These include relatively slow to
@@ -425,7 +472,7 @@ class ViewImpl {
auto metadata() const -> uint8_t* {
return reinterpret_cast<uint8_t*>(storage_);
}
auto entries() const -> EntryT* {
auto entries_data() const -> EntryT* {
return reinterpret_cast<EntryT*>(reinterpret_cast<std::byte*>(storage_) +
EntriesOffset(alloc_size_));
}
@@ -457,6 +504,172 @@ class ViewImpl {
Storage* storage_;
};
// A range over the entries of a hashtable.
//
// A dedicated range object is used rather than a plain pair of iterators (such
// as `llvm::iterator_range`) because the range scopes two debug-only behaviors
// that a bare iterator pair has nowhere to store:
//
// - Mutation checking: the range snapshots a hash of the table's metadata on
// construction and re-checks it on destruction, catching tables that were
// mutated while iteration was active.
// - Traversal order: the group at which iteration starts, and the stride it
// walks the groups with, are drawn from an entropy pool once when the range
// is constructed.
// Deriving them here rather than in `begin()` keeps `begin()` a pure function
// of the range so that it can be called repeatedly, as forward ranges
// require, while still varying the order between separately created ranges.
//
// The range holds the view *by value*; views are two words and designed to be
// cheap to copy. It deliberately does not point back at the view it was created
// from, as views are routinely temporaries or by-value parameters whose
// lifetime is shorter than the table they refer to.
//
// This type provides only the minimal `begin()` and `end()` interface needed by
// range-based for loops and the range concepts, which also avoids any
// compile-time cost from including `<ranges>`.
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
class ViewImpl<InputKeyT, InputValueT, InputKeyContextT>::EntryRange {
public:
class Iterator;
using value_type = typename EntryT::IterValueT;
using reference = EntryRefT;
using difference_type = ssize_t;
explicit EntryRange(ViewImpl view);
// Copyable: every member is a scalar snapshot of the table. Copying a range
// in a debug build simply validates the same table state more than once.
EntryRange(const EntryRange&) = default;
auto operator=(const EntryRange&) -> EntryRange& = default;
#ifndef NDEBUG
// Only debug builds declare a destructor, and so only they re-check the
// table on the way out. Release builds leave the range trivially
// destructible, and so trivial for the purposes of calls, letting it be
// passed and returned in registers.
~EntryRange() { CheckInvariants(); }
#endif
auto begin() const -> Iterator;
auto end() const -> Iterator;
private:
// The facade `Iterator` derives from. A class can't name one of its own
// aliases in its base-specifier, so naming it here lets `Iterator` spell it
// once instead of repeating it to get at the members it inherits.
using IteratorBase =
llvm::iterator_facade_base<Iterator, typename EntryT::IterCategoryT,
value_type, difference_type,
typename EntryT::IterPointerT, reference>;
#ifndef NDEBUG
// Checks that the table's metadata has not changed since construction.
auto CheckInvariants() const -> void;
#endif
ViewImpl view_;
#ifndef NDEBUG
HashCode initial_metadata_hash_ = {};
ssize_t start_group_ = 0;
ssize_t step_ = GroupSize;
#endif
};
// Two-level forward iterator through present hashtable entries.
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
class ViewImpl<InputKeyT, InputValueT, InputKeyContextT>::EntryRange::Iterator
: public EntryRange::IteratorBase {
public:
// Both the set and map forms satisfy C++20's `std::forward_iterator`. A
// map's `reference` is a proxy, which pins its C++17 `iterator_category` to
// `input`, but the C++20 concept is unaffected. See `EntryRefT`.
using iterator_concept = std::forward_iterator_tag;
Iterator() = default;
using EntryRange::IteratorBase::operator++;
[[clang::always_inline]] auto operator*() const -> EntryRefT {
CARBON_DCHECK(present_bits_ != 0, "Dereferencing end iterator!");
__builtin_assume(present_bits_ != 0);
// `index_ptr` folds scaling the match index by the entry size together
// with decoding the index itself, which saves a shift on the portable
// byte-encoded code path.
return MatchIndex(present_bits_).index_ptr(group_entries())->ref();
}
[[clang::always_inline]] auto operator++() -> Iterator& {
CARBON_DCHECK(present_bits_ != 0, "Incrementing end iterator!");
__builtin_assume(present_bits_ != 0);
present_bits_ &= (present_bits_ - 1);
if (LLVM_LIKELY(present_bits_ != 0)) {
return *this;
}
AdvanceToNextPresentGroup();
return *this;
}
friend auto operator==(const Iterator& lhs, const Iterator& rhs) -> bool {
if (lhs.present_bits_ == 0 || rhs.present_bits_ == 0) {
return lhs.present_bits_ == rhs.present_bits_;
}
// The entry pointer already encodes the base and the group offset, so it
// uniquely identifies the group without a separate index.
return lhs.group_entries() == rhs.group_entries() &&
lhs.present_bits_ == rhs.present_bits_;
}
private:
friend class EntryRange;
using MatchBitsT = typename MetadataGroup::MatchPresentRange::BitsT;
using MatchIndex = typename MetadataGroup::MatchIndex;
// Builds an iterator to the first present entry of `range`, or an iterator
// equal to `end()` when the range has no entries to walk. The parameters of
// the walk differ between builds, so both are drawn from the range here
// rather than passed in.
[[clang::always_inline]] explicit Iterator(const EntryRange& range);
[[clang::always_inline]] auto AdvanceToNextPresentGroup() -> void;
// The entries of the group the iterator is currently within. Both builds
// track the current group, but they encode it differently, so the encoding
// is hidden behind this accessor.
auto group_entries() const -> EntryT* {
#ifndef NDEBUG
return group_entries_;
#else
return entries_end_ + group_offset_;
#endif
}
#ifndef NDEBUG
// Debug builds walk groups in a randomized order and so must retain the
// array bases along with the parameters of the walk. The randomized walk
// revisits no group but also never reaches the end of the array, so it does
// need an explicit count of the groups left to visit.
EntryT* group_entries_ = nullptr;
const uint8_t* metadata_ = nullptr;
EntryT* entries_ = nullptr;
ssize_t groups_remaining_ = 0;
ssize_t group_index_ = 0;
size_t probe_mask_ = 0;
ssize_t step_ = GroupSize;
#else
// Release builds walk the groups in order, tracking the position as a
// *negative* byte offset from the end of each array that counts up to zero.
// Anchoring at the ends rather than the beginnings means the walk needs only
// this one induction variable, and reaching zero is the bound.
EntryT* entries_end_ = nullptr;
const uint8_t* metadata_end_ = nullptr;
ssize_t group_offset_ = 0;
#endif
MatchBitsT present_bits_ = 0;
};
// Implementation helper for defining a read-write base type for a hashtable
// that type-erases any SSO buffer.
//
@@ -495,7 +708,10 @@ class BaseImpl {
// NOLINTNEXTLINE(google-explicit-constructor): Designed to implicitly decay.
explicit(false) operator ViewImplT() const { return view_impl(); }
auto view_impl() const -> ViewImplT { return view_impl_; }
auto view_impl() const -> const ViewImplT& { return view_impl_; }
// Destroys all non-trivially destructible entries in the table.
auto DestroyEntries() -> void;
// Looks up the provided key in the hashtable. If found, returns a pointer to
// that entry and `false`.
@@ -510,7 +726,7 @@ class BaseImpl {
// Grow the table to specific allocation size.
//
// This will grow the the table if necessary for it to have an allocation size
// This will grow the table if necessary for it to have an allocation size
// of `target_alloc_size` which must be a power of two. Note that this will
// not allow that many keys to be inserted into the hashtable, but a smaller
// number based on the load factor. If a specific number of insertions need to
@@ -561,7 +777,7 @@ class BaseImpl {
auto storage() const -> Storage* { return view_impl_.storage_; }
auto storage() -> Storage*& { return view_impl_.storage_; }
auto metadata() const -> uint8_t* { return view_impl_.metadata(); }
auto entries() const -> EntryT* { return view_impl_.entries(); }
auto entries_data() const -> EntryT* { return view_impl_.entries_data(); }
auto small_alloc_size() const -> ssize_t {
return static_cast<unsigned>(small_alloc_size_);
}
@@ -665,6 +881,25 @@ inline auto ComputeSeed() -> uint64_t {
return reinterpret_cast<uint64_t>(&global_addr_seed);
}
#ifndef NDEBUG
// A pool of entropy used to vary the iteration order of hashtables in debug
// builds. It is seeded from ASLR where available.
extern std::atomic<HashCode> entropy_hash;
// Returns a pseudo-random value from the entropy pool, advancing the pool.
//
// The load and store are separate relaxed operations rather than one atomic
// read-modify-write so that consuming entropy is just a load, and refreshing
// the pool doesn't block the iteration that follows. Racing callers can lose an
// update and draw the same value, which is fine for a debug aid.
inline auto NextRangeEntropy() -> HashCode {
HashCode prev_entropy_hash = entropy_hash.load(std::memory_order_relaxed);
entropy_hash.store(Carbon::HashValue(prev_entropy_hash),
std::memory_order_relaxed);
return prev_entropy_hash;
}
#endif
inline auto ComputeProbeMaskFromSize(ssize_t size) -> size_t {
CARBON_DCHECK(llvm::isPowerOf2_64(size),
"Size must be a power of two for a hashed buffer!");
@@ -748,7 +983,7 @@ auto ViewImpl<InputKeyT, InputValueT, InputKeyContextT>::LookupEntry(
HashCode hash = key_context.HashKey(lookup_key, ComputeSeed());
auto [hash_index, tag] = hash.ExtractIndexAndTag<7>();
EntryT* local_entries = entries();
EntryT* local_entries = entries_data();
// Walk through groups of entries using a quadratic probe starting from
// `hash_index`.
@@ -799,41 +1034,11 @@ auto ViewImpl<InputKeyT, InputValueT, InputKeyContextT>::LookupEntry(
} while (LLVM_UNLIKELY(true));
}
// Note that we force inlining here because we expect to be called with lambdas
// that will in turn be inlined to form the loop body. We don't want function
// boundaries within the loop for performance, and recognizing the degree of
// simplification from inlining these callbacks may be difficult to
// automatically recognize.
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
template <typename EntryCallbackT, typename GroupCallbackT>
[[clang::always_inline]] auto
ViewImpl<InputKeyT, InputValueT, InputKeyContextT>::ForEachEntry(
EntryCallbackT entry_callback, GroupCallbackT group_callback) const
-> void {
uint8_t* local_metadata = metadata();
EntryT* local_entries = entries();
ssize_t local_size = alloc_size_;
for (ssize_t group_index = 0; group_index < local_size;
group_index += GroupSize) {
auto g = MetadataGroup::Load(local_metadata, group_index);
auto present_matched_range = g.MatchPresent();
if (!present_matched_range) {
continue;
}
for (ssize_t byte_index : present_matched_range) {
entry_callback(local_entries[group_index + byte_index]);
}
group_callback(&local_metadata[group_index]);
}
}
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
auto ViewImpl<InputKeyT, InputValueT, InputKeyContextT>::ComputeMetricsImpl(
KeyContextT key_context) const -> Metrics {
uint8_t* local_metadata = metadata();
EntryT* local_entries = entries();
EntryT* local_entries = entries_data();
ssize_t local_size = alloc_size_;
Metrics metrics;
@@ -898,6 +1103,147 @@ auto ViewImpl<InputKeyT, InputValueT, InputKeyContextT>::ComputeMetricsImpl(
return metrics;
}
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
[[clang::always_inline]] auto
ViewImpl<InputKeyT, InputValueT, InputKeyContextT>::EntriesImpl() const
-> EntryRange {
return EntryRange(*this);
}
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
[[clang::always_inline]]
ViewImpl<InputKeyT, InputValueT, InputKeyContextT>::EntryRange::Iterator::
Iterator(const EntryRange& range) {
const ViewImpl& view = range.view_;
ssize_t alloc_size = view.alloc_size_;
// An empty or moved-from table has no groups to load from, and the
// default-initialized state left behind already compares equal to `end()`.
if (alloc_size == 0 || view.storage_ == nullptr) {
return;
}
#ifndef NDEBUG
entries_ = view.entries_data();
metadata_ = view.metadata();
// The starting group and stride were drawn when the range was constructed,
// so every iterator built from it walks the same order.
group_index_ = range.start_group_;
group_entries_ = entries_ + group_index_;
groups_remaining_ = alloc_size / GroupSize - 1;
probe_mask_ = ComputeProbeMaskFromSize(alloc_size);
step_ = range.step_;
auto g = MetadataGroup::Load(metadata_, group_index_);
#else
// The allocation size bounds the metadata array directly, so anchoring at
// the ends of the arrays lets the walk run off a single induction variable
// without ever dividing by the group size.
entries_end_ = view.entries_data() + alloc_size;
metadata_end_ = view.metadata() + alloc_size;
group_offset_ = -alloc_size;
auto g = MetadataGroup::Load(metadata_end_, group_offset_);
#endif
auto present_range = g.MatchPresent();
if (present_range) {
present_bits_ = static_cast<MatchBitsT>(present_range);
} else {
AdvanceToNextPresentGroup();
}
}
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
[[clang::always_inline]] auto
ViewImpl<InputKeyT, InputValueT,
InputKeyContextT>::EntryRange::Iterator::AdvanceToNextPresentGroup()
-> void {
#ifndef NDEBUG
while (--groups_remaining_ >= 0) {
group_index_ = static_cast<ssize_t>(
static_cast<size_t>(group_index_ + step_) & probe_mask_);
auto g = MetadataGroup::Load(metadata_, group_index_);
auto range = g.MatchPresent();
if (range) {
group_entries_ = entries_ + group_index_;
present_bits_ = static_cast<MatchBitsT>(range);
return;
}
}
#else
for (group_offset_ += GroupSize; group_offset_ != 0;
group_offset_ += GroupSize) {
auto g = MetadataGroup::Load(metadata_end_, group_offset_);
auto range = g.MatchPresent();
if (range) {
present_bits_ = static_cast<MatchBitsT>(range);
return;
}
}
#endif
present_bits_ = 0;
}
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
ViewImpl<InputKeyT, InputValueT, InputKeyContextT>::EntryRange::EntryRange(
ViewImpl view)
: view_(view) {
#ifndef NDEBUG
if (view_.alloc_size_ <= 0 || view_.storage_ == nullptr) {
return;
}
initial_metadata_hash_ = Carbon::HashValue(
llvm::ArrayRef<uint8_t>(view_.metadata(), view_.alloc_size_));
// Draw the traversal order once, here, so that `begin()` remains a pure
// function of the range and can be called repeatedly. Two separately
// constructed ranges still walk the table in different orders.
start_group_ = NextRangeEntropy().ExtractIndex() &
ComputeProbeMaskFromSize(view_.alloc_size_);
// Walk the groups with a stride of an odd number of groups. The group count
// is always a power of two, so any odd stride is coprime with it and visits
// every group exactly once before repeating. That scrambles the group order
// far more thoroughly than a forward or reverse scan, and costs nothing in
// the loop itself as the increment already adds a stride and masks.
ssize_t num_groups = view_.alloc_size_ / GroupSize;
ssize_t stride_groups =
(NextRangeEntropy().ExtractIndex() & (num_groups - 1)) | 1;
step_ = stride_groups * GroupSize;
#endif
}
#ifndef NDEBUG
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
auto ViewImpl<InputKeyT, InputValueT,
InputKeyContextT>::EntryRange::CheckInvariants() const -> void {
if (view_.alloc_size_ <= 0 || view_.storage_ == nullptr) {
return;
}
HashCode current_hash = Carbon::HashValue(
llvm::ArrayRef<uint8_t>(view_.metadata(), view_.alloc_size_));
CARBON_CHECK(current_hash == initial_metadata_hash_,
"Hashtable mutated during iteration: metadata changed!");
}
#endif
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
[[clang::always_inline]] auto
ViewImpl<InputKeyT, InputValueT, InputKeyContextT>::EntryRange::begin() const
-> Iterator {
// The traversal order is fixed when the range is constructed, so repeated
// calls yield equal iterators as forward ranges require.
return Iterator(*this);
}
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
[[clang::always_inline]] auto
ViewImpl<InputKeyT, InputValueT, InputKeyContextT>::EntryRange::end() const
-> Iterator {
return Iterator();
}
// TODO: Evaluate whether it is worth forcing this out-of-line given the
// reasonable ABI boundary it forms and large volume of code necessary to
// implement it.
@@ -921,7 +1267,7 @@ auto BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::InsertImpl(
ssize_t group_with_deleted_index;
MetadataGroup::MatchIndex deleted_match = {};
EntryT* local_entries = entries();
EntryT* local_entries = entries_data();
auto return_insert_at_index = [&](ssize_t index) -> std::pair<EntryT*, bool> {
// We'll need to insert at this index so set the control group byte to the
@@ -1017,7 +1363,7 @@ BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::GrowToAllocSizeImpl(
bool old_small = is_small();
Storage* old_storage = storage();
uint8_t* old_metadata = metadata();
EntryT* old_entries = entries();
EntryT* old_entries = entries_data();
// Configure for the new size and allocate the new storage.
alloc_size() = target_alloc_size;
@@ -1093,7 +1439,7 @@ auto BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::EraseImpl(
// If we mark the slot as empty, we'll also need to increase the growth
// budget.
uint8_t* local_metadata = metadata();
EntryT* local_entries = entries();
EntryT* local_entries = entries_data();
ssize_t index = entry - local_entries;
ssize_t group_index = index & ~GroupMask;
auto g = MetadataGroup::Load(local_metadata, group_index);
@@ -1114,16 +1460,10 @@ auto BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::EraseImpl(
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
auto BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::ClearImpl() -> void {
view_impl_.ForEachEntry(
[](EntryT& entry) {
if constexpr (!EntryT::IsTriviallyDestructible) {
entry.Destroy();
}
},
[](uint8_t* metadata_group) {
// Clear the group.
std::memset(metadata_group, 0, GroupSize);
});
DestroyEntries();
if (storage() != nullptr) {
std::memset(metadata(), 0, alloc_size());
}
growth_budget_ = GrowthThresholdForAllocSize(alloc_size());
}
@@ -1186,10 +1526,7 @@ auto BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::Destroy() -> void {
}
// Destroy all the entries.
if constexpr (!EntryT::IsTriviallyDestructible) {
view_impl_.ForEachEntry([](EntryT& entry) { entry.Destroy(); },
[](auto...) {});
}
DestroyEntries();
// If small, nothing to deallocate.
if (is_small()) {
@@ -1201,6 +1538,16 @@ auto BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::Destroy() -> void {
Deallocate(storage(), alloc_size());
}
template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
auto BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::DestroyEntries()
-> void {
if constexpr (!EntryT::IsTriviallyDestructible) {
for (typename EntryT::RefT entry : view_impl_.EntriesImpl()) {
EntryT::DestroyRef(entry);
}
}
}
// Copy all of the slots over from another table that is exactly the same
// allocation size.
//
@@ -1224,9 +1571,9 @@ auto BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::CopySlotsFrom(
// all of the keys. This is especially important as we don't have an easy way
// to access the key context needed for rehashing here.
uint8_t* local_metadata = metadata();
EntryT* local_entries = entries();
EntryT* local_entries = entries_data();
const uint8_t* local_arg_metadata = arg.metadata();
const EntryT* local_arg_entries = arg.entries();
const EntryT* local_arg_entries = arg.entries_data();
memcpy(local_metadata, local_arg_metadata, local_size);
for (ssize_t group_index = 0; group_index < local_size;
@@ -1269,9 +1616,9 @@ auto BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::MoveFrom(
// themselves. We do this preserving their slots and even tombstones to
// avoid rehashing.
uint8_t* local_metadata = this->metadata();
EntryT* local_entries = this->entries();
EntryT* local_entries = this->entries_data();
uint8_t* local_arg_metadata = arg.metadata();
EntryT* local_arg_entries = arg.entries();
EntryT* local_arg_entries = arg.entries_data();
memcpy(local_metadata, local_arg_metadata, local_size);
if (EntryT::IsTriviallyRelocatable) {
memcpy(local_entries, local_arg_entries, local_size * sizeof(EntryT));
@@ -1306,7 +1653,7 @@ auto BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::InsertIntoEmpty(
HashCode hash) -> EntryT* {
auto [hash_index, tag] = hash.ExtractIndexAndTag<7>();
uint8_t* local_metadata = metadata();
EntryT* local_entries = entries();
EntryT* local_entries = entries_data();
for (ProbeSequence s(hash_index, alloc_size());; s.Next()) {
ssize_t group_index = s.index();
@@ -1392,7 +1739,7 @@ auto BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::GrowToNextAllocSize(
bool old_small = is_small();
Storage* old_storage = storage();
uint8_t* old_metadata = metadata();
EntryT* old_entries = entries();
EntryT* old_entries = entries_data();
#ifndef NDEBUG
// Count how many of the old table slots will end up being empty after we grow
@@ -1417,7 +1764,7 @@ auto BaseImpl<InputKeyT, InputValueT, InputKeyContextT>::GrowToNextAllocSize(
// Now extract the new components of the table.
uint8_t* new_metadata = metadata();
EntryT* new_entries = entries();
EntryT* new_entries = entries_data();
// Walk the metadata groups, clearing deleted to empty, duplicating the
// metadata for the low and high halves, and updating it based on where each
@@ -1596,10 +1943,7 @@ auto TableImpl<InputBaseT, SmallSize>::operator=(const TableImpl& arg)
return *this;
}
CARBON_DCHECK(arg.storage() != this->storage());
if constexpr (!EntryT::IsTriviallyDestructible) {
this->view_impl_.ForEachEntry([](EntryT& entry) { entry.Destroy(); },
[](auto...) {});
}
this->DestroyEntries();
} else {
// The sizes don't match so destroy everything and re-setup the table
// storage.
+19 -20
View File
@@ -7,6 +7,7 @@
#include <concepts>
#include <type_traits>
#include <utility>
#include "common/check.h"
#include "common/hashtable_key_context.h"
@@ -60,6 +61,10 @@ class SetView : RawHashtable::ViewImpl<InputKeyT, void, InputKeyContextT> {
using KeyContextT = ImplT::KeyContextT;
using MetricsT = ImplT::MetricsT;
// A range over the keys of the set. Bound to the lifetime of the viewed set,
// and invalidated by mutating it.
using Range = ImplT::EntryRange;
// This type represents the result of lookup operations. It encodes whether
// the lookup was a success as well as accessors for the key.
class LookupResult {
@@ -91,10 +96,8 @@ class SetView : RawHashtable::ViewImpl<InputKeyT, void, InputKeyContextT> {
auto Lookup(LookupKeyT lookup_key,
KeyContextT key_context = KeyContextT()) const -> LookupResult;
// Run the provided callback for every key in the set.
template <typename CallbackT>
auto ForEach(CallbackT callback) const -> void
requires(std::invocable<CallbackT, KeyT&>);
// Returns a range for iterating over all keys in the set.
auto entries() const -> Range;
// This routine is relatively inefficient and only intended for use in
// benchmarking or logging of performance anomalies. The specific metrics
@@ -143,6 +146,7 @@ class SetBase
using ViewT = SetView<KeyT, KeyContextT>;
using LookupResult = ViewT::LookupResult;
using MetricsT = ImplT::MetricsT;
using Range = ViewT::Range;
// The result type for insertion operations both indicates whether an insert
// was needed (as opposed to the key already being in the set), and provides
@@ -190,12 +194,12 @@ class SetBase
}
// Convenience forwarder to the view type.
template <typename CallbackT>
auto ForEach(CallbackT callback) const -> void
requires(std::invocable<CallbackT, KeyT&>)
{
return ViewT(*this).ForEach(callback);
}
auto entries() const& -> Range { return ViewT(*this).entries(); }
// Deleted on rvalues: the range refers to storage owned by this table, so a
// range built from a temporary set would dangle. Both qualifiers are needed
// as `&&` alone would leave a const rvalue binding to the `const&` overload.
auto entries() && = delete;
auto entries() const&& = delete;
// Convenience forwarder to the view type.
auto ComputeMetrics(KeyContextT key_context = KeyContextT()) const
@@ -211,10 +215,10 @@ class SetBase
auto Insert(LookupKeyT lookup_key, KeyContextT key_context = KeyContextT())
-> InsertResult;
// Insert a key into the map and call the provided callback if necessary to
// produce a new key when no existing value is found.
// Insert a key into the set and call the provided callback if necessary to
// produce a new key when no existing key is found.
//
// Example: `m.Insert(key_equivalent, [] { return real_key; });`
// Example: `s.Insert(key_equivalent, [] { return real_key; });`
//
// The point of this function is when the lookup key is _different_from the
// stored key. However, we don't restrict it in case that blocks generic
@@ -333,13 +337,8 @@ auto SetView<InputKeyT, InputKeyContextT>::Lookup(LookupKeyT lookup_key,
}
template <typename InputKeyT, typename InputKeyContextT>
template <typename CallbackT>
auto SetView<InputKeyT, InputKeyContextT>::ForEach(CallbackT callback) const
-> void
requires(std::invocable<CallbackT, KeyT&>)
{
this->ForEachEntry([callback](EntryT& entry) { callback(entry.key()); },
[](auto...) {});
auto SetView<InputKeyT, InputKeyContextT>::entries() const -> Range {
return this->ImplT::EntriesImpl();
}
template <typename InputKeyT, typename InputKeyContextT>
+70 -1
View File
@@ -35,8 +35,9 @@ static constexpr bool IsCarbonSet = IsCarbonSetImpl<SetT>::value;
// support different APIs. The primary template assumes a roughly
// `std::unordered_set` API design, and types with a different API design are
// supported through specializations.
template <typename SetT>
template <typename InSetT>
struct SetWrapperImpl {
using SetT = InSetT;
using KeyT = SetT::key_type;
SetT s;
@@ -58,6 +59,17 @@ struct SetWrapperImpl {
}
auto BenchErase(KeyT k) -> bool { return s.erase(k) != 0; }
// Visits every key in the set, calling `cb` with each one. Each set type is
// expected to traverse using whatever API it provides for this, so that the
// benchmark measures iterating the set rather than any specific iteration
// API.
template <typename CallbackT>
auto BenchIterate(CallbackT cb) -> void {
for (const auto& k : s) {
cb(k);
}
}
};
// Explicit (partial) specialization for the Carbon map type that uses its
@@ -85,6 +97,13 @@ struct SetWrapperImpl<Set<KT, MinSmallSize>> {
}
auto BenchErase(KeyT k) -> bool { return s.Erase(k); }
template <typename CallbackT>
auto BenchIterate(CallbackT cb) -> void {
for (const auto& k : s.entries()) {
cb(k);
}
}
};
// Provide a way to override the Carbon Set specific benchmark runs with another
@@ -123,6 +142,17 @@ using SetWrapper =
SetWrapperOverride<SetT, SetOverride::CARBON_SET_BENCH_OVERRIDE>;
#endif
// Reports extra statistics about the table, when it is in fact a Carbon table.
// Note that this has to inspect the *wrapped* type in order to work correctly
// when the Carbon benchmarks are overridden with another implementation.
template <typename SetT>
auto ReportMetrics(const SetWrapper<SetT>& s_wrapper, benchmark::State& state)
-> void {
if constexpr (IsCarbonSet<typename SetWrapper<SetT>::SetT>) {
ReportTableMetrics(s_wrapper.s, state);
}
}
// NOLINTBEGIN(bugprone-macro-parentheses): Parentheses are incorrect here.
#define MAP_BENCHMARK_ONE_OP_SIZE(NAME, APPLY, KT) \
BENCHMARK(NAME<Set<KT>>)->Apply(APPLY); \
@@ -375,5 +405,44 @@ static void BM_SetInsertSeq(benchmark::State& state) {
}
MAP_BENCHMARK_OP_SEQ(BM_SetInsertSeq);
// Benchmark visiting every key in a set.
//
// Unlike the lookup benchmarks, this walks the table's storage from end to end
// rather than probing it, so it is largely a measure of how densely keys are
// packed and how cheaply empty slots can be skipped. There is no dependency
// between the keys visited, and so this is a throughput measurement.
//
// Each batch is a single complete traversal of the set, with the batch size set
// to the number of keys so that the reported time is the per-key cost.
template <typename SetT>
static void BM_SetIterate(benchmark::State& state) {
using SetWrapperT = SetWrapper<SetT>;
using KT = typename SetWrapperT::KeyT;
SetWrapperT s;
auto [keys, _] = GetKeysAndMissKeys<KT>(state.range(0));
for (auto k : keys) {
bool inserted = s.BenchInsert(k);
CARBON_DCHECK(inserted, "Must be a successful insert!");
}
while (state.KeepRunningBatch(keys.size())) {
ssize_t sum = 0;
s.BenchIterate([&sum](const KT& k) {
// Consume the key so that neither the traversal nor the loads out of the
// entries can be optimized away.
sum += ValueToBool(k);
});
benchmark::DoNotOptimize(sum);
}
// The time is already per-key, so an iteration-invariant rate of one gives
// the throughput of keys visited.
state.counters["KeyRate"] =
benchmark::Counter(1, benchmark::Counter::kIsIterationInvariantRate);
ReportMetrics(s, state);
}
MAP_BENCHMARK_ONE_OP(BM_SetIterate, SizeArgs);
} // namespace
} // namespace Carbon
+181 -1
View File
@@ -7,7 +7,12 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <concepts>
#include <initializer_list>
#include <iterator>
#include <ranges>
#include <set>
#include <string>
#include <type_traits>
#include <vector>
@@ -19,6 +24,7 @@ namespace {
using RawHashtable::IndexKeyContext;
using RawHashtable::MoveOnlyTestData;
using RawHashtable::TestData;
using ::testing::UnorderedElementsAre;
using ::testing::UnorderedElementsAreArray;
template <typename SetT, typename MatcherRangeT>
@@ -26,7 +32,9 @@ auto ExpectSetElementsAre(SetT&& s, MatcherRangeT element_matchers) -> void {
// Collect the elements into a container.
using KeyT = std::remove_reference<SetT>::type::KeyT;
std::vector<std::reference_wrapper<KeyT>> entries;
s.ForEach([&entries](KeyT& k) { entries.push_back(std::ref(k)); });
for (auto& k : s.entries()) {
entries.push_back(std::ref(k));
}
// Use the GoogleMock unordered container matcher to validate and show errors
// on wrong elements.
@@ -176,6 +184,8 @@ TYPED_TEST(SetTest, Move) {
SetT other_s1 = std::move(s);
ExpectSetElementsAre(other_s1, MakeElements(llvm::seq(1, 24)));
// A moved-from set has a size but no storage, and must iterate as empty.
EXPECT_EQ(s.entries().begin(), s.entries().end());
// Add some more elements.
for (int i : llvm::seq(24, 32)) {
@@ -432,5 +442,175 @@ TEST(SetContextTest, Basic) {
ExpectSetElementsAre(s, MakeElements(llvm::seq(1, 512)));
}
TYPED_TEST(SetTest, Range) {
using SetT = TypeParam;
using Range = decltype(std::declval<const SetT&>().entries());
using Iter = typename Range::Iterator;
static_assert(std::forward_iterator<Iter>);
static_assert(std::same_as<decltype(std::declval<Range>().begin()), Iter>);
static_assert(std::same_as<decltype(std::declval<Range>().end()), Iter>);
static_assert(std::ranges::forward_range<Range>);
static_assert(std::ranges::common_range<Range>);
SetT s;
EXPECT_EQ(s.entries().begin(), s.entries().end());
for (const auto& k : s.entries()) {
static_cast<void>(k);
FAIL() << "Empty set range should have no elements";
}
for (int i = 1; i <= 5; ++i) {
s.Insert(i);
}
// Range-for traversal by const ref.
int count = 0;
for (const auto& k : s.entries()) {
EXPECT_GE(k, 1);
EXPECT_LE(k, 5);
++count;
}
EXPECT_EQ(count, 5);
// Direct GMock container matching.
EXPECT_THAT(s.entries(), UnorderedElementsAre(1, 2, 3, 4, 5));
// Const view range iteration.
using KeyT = typename SetT::KeyT;
using KeyContextT = typename SetT::KeyContextT;
SetView<const KeyT, KeyContextT> cv = s;
int cv_count = 0;
for (const auto& k : cv.entries()) {
static_assert(std::is_const_v<std::remove_reference_t<decltype(k)>>);
EXPECT_GE(k, 1);
EXPECT_LE(k, 5);
++cv_count;
}
EXPECT_EQ(cv_count, 5);
EXPECT_THAT(cv.entries(), UnorderedElementsAre(1, 2, 3, 4, 5));
// Explicit iterator traversal, dereference, and post-increment.
auto r = s.entries();
int iter_count = 0;
for (auto it = r.begin(); it != r.end(); ++it) {
EXPECT_NE(*it, 0);
++iter_count;
}
EXPECT_EQ(iter_count, 5);
auto it = r.begin();
auto prev = it++;
EXPECT_NE(it, prev);
}
TYPED_TEST(MoveOnlySetTest, Range) {
TypeParam s;
s.Insert(1);
s.Insert(2);
int count = 0;
for (const auto& k : s.entries()) {
EXPECT_GT(k.value, 0);
++count;
}
EXPECT_EQ(count, 2);
}
#ifndef NDEBUG
TEST(SetDeathTest, MutateDuringIterationFails) {
EXPECT_DEATH(([] {
Set<int> s;
s.Insert(1);
auto range = s.entries();
s.Insert(2);
}()),
"Hashtable mutated during iteration");
}
#endif
// A range outlives the *view* it was built from: views don't own storage, and
// the range copies the view rather than pointing at it.
TEST(SetTest, RangeOutlivesTemporaryView) {
Set<int> s;
s.Insert(1);
auto make_view = [&s]() -> SetView<int> { return s; };
auto range = make_view().entries();
EXPECT_THAT(range, UnorderedElementsAre(1));
}
#ifdef NDEBUG
// Release iteration state is two end pointers, a group offset, and the
// present-bit mask; it needs to stay small enough to live in registers across
// the loop. Debug builds add the randomized walk and mutation-check state.
static_assert(sizeof(Set<int>::Range::Iterator) <= 4 * sizeof(void*));
#endif
// Forward ranges guarantee multi-pass: `begin()` must be a pure function of the
// range. Debug builds draw their traversal entropy when the range is
// constructed rather than in `begin()` precisely so that repeated calls start
// from the same group.
TEST(SetTest, RangeIsMultiPass) {
Set<int, 16> s;
for (int i = 1; i <= 64; ++i) {
s.Insert(i);
}
auto range = s.entries();
EXPECT_EQ(range.begin(), range.begin());
// Two passes over the same range must agree on both the keys visited and the
// order they're visited in.
std::vector<int> first;
for (int k : range) {
first.push_back(k);
}
std::vector<int> second;
for (int k : range) {
second.push_back(k);
}
EXPECT_EQ(first, second);
EXPECT_EQ(static_cast<ssize_t>(first.size()), 64);
}
// Whatever order a range picks, it has to be a genuine permutation of the
// table. Debug builds additionally vary that order between ranges over the same
// table so that callers can't come to depend on it.
TEST(SetTest, TraversalOrderIsAVaryingPermutation) {
Set<int, 64> s;
std::vector<int> inserted;
// Enough keys to populate every group of the small storage.
for (int i = 0; i < 36; ++i) {
int key = i * 17 + 7;
EXPECT_TRUE(s.Insert(key).is_inserted());
inserted.push_back(key);
}
std::set<std::vector<int>> distinct_orders;
for (int i = 0; i < 64; ++i) {
std::vector<int> visited;
for (int k : s.entries()) {
visited.push_back(k);
}
// A walk that skipped a group would drop keys and one that revisited a
// group would duplicate them, so comparing as a multiset covers both. This
// is what makes an odd group stride a valid traversal.
EXPECT_THAT(visited, UnorderedElementsAreArray(inserted));
distinct_orders.insert(visited);
}
#ifndef NDEBUG
// Debug builds randomize both the starting group and the stride, so across
// this many ranges we should see more than the two orders (pure forward and
// pure reverse) that a simple direction flip would produce.
EXPECT_GT(distinct_orders.size(), 2)
<< "Debug traversal order does not appear to be randomized.";
#else
// Release builds always scan the groups in order.
EXPECT_EQ(distinct_orders.size(), 1);
#endif
}
} // namespace
} // namespace Carbon
+6 -9
View File
@@ -554,16 +554,13 @@ auto SourceGen::AppendUniqueIdentifiers(
// Append all the identifiers directly out of the set. We make no guarantees
// about the relative order so we just use the non-deterministic order of the
// set and avoid additional storage.
//
// TODO: It's awkward the `ForEach` here can't early-exit. This just walks the
// whole set which is harmless if inefficient. We should add early exiting
// the loop support to `Set` and update this code.
unique_idents.ForEach([&](llvm::StringRef ident) {
if (number > 0) {
dest.push_back(ident);
--number;
for (llvm::StringRef ident : unique_idents.entries()) {
if (number == 0) {
break;
}
});
dest.push_back(ident);
--number;
}
CARBON_CHECK(number == 0);
}
+4 -4
View File
@@ -143,7 +143,7 @@ auto ScopeStack::Pop(bool check_unused) -> void {
// TODO: Multiple diagnostics on same line has non-deterministic order.
// Add second sort key in diagnostics sorting.
scope.names.ForEach([&, check_unused](SemIR::NameId name_id) {
for (SemIR::NameId name_id : scope.names.entries()) {
auto& lexical_results = lexical_lookup_.Get(name_id);
CARBON_CHECK(lexical_results.back().scope_index == scope.index,
"Inconsistent scope index for name {0}", name_id);
@@ -151,7 +151,7 @@ auto ScopeStack::Pop(bool check_unused) -> void {
CheckUnusedBinding(*context_, name_id, lexical_results.back());
}
lexical_results.pop_back();
});
}
if (!scope.is_lexical_scope()) {
CARBON_CHECK(non_lexical_scope_stack_.back().scope_index == scope.index);
@@ -375,7 +375,7 @@ auto ScopeStack::Suspend() -> SuspendedScope {
result.suspended_items.reserve(result.entry.num_names +
peek_compile_time_bindings.size());
result.entry.names.ForEach([&](SemIR::NameId name_id) {
for (SemIR::NameId name_id : result.entry.names.entries()) {
auto suspended = lexical_lookup_.Suspend(name_id);
CARBON_CHECK(suspended.index !=
SuspendedScope::ScopeItem::IndexForCompileTimeBinding);
@@ -384,7 +384,7 @@ auto ScopeStack::Suspend() -> SuspendedScope {
.inst_id = suspended.inst_id,
.is_decl_reachable = suspended.is_decl_reachable,
.use_loc_id = suspended.use_loc_id});
});
}
CARBON_CHECK(static_cast<int>(result.suspended_items.size()) ==
result.entry.num_names);
+6 -7
View File
@@ -70,13 +70,12 @@ auto SpecificCoalescer::CoalesceEquivalentSpecifics(
// When processing equivalences, we may change the canonical specific
// multiple times, so we don't delete replaced specifics until the
// end.
visited_equivalent_specifics.ForEach(
[&](std::pair<SemIR::SpecificId, SemIR::SpecificId>
equivalent_entry) {
CARBON_VLOG("Found equivalent specifics: {0}, {1}",
equivalent_entry.first, equivalent_entry.second);
ProcessSpecificEquivalence(equivalent_entry);
});
for (const auto& equivalent_entry :
visited_equivalent_specifics.entries()) {
CARBON_VLOG("Found equivalent specifics: {0}, {1}",
equivalent_entry.first, equivalent_entry.second);
ProcessSpecificEquivalence(equivalent_entry);
}
// Removed the replaced specific from the list of emitted specifics.
// Only the top level, since the others are somewhere else in the
+3 -2
View File
@@ -72,8 +72,9 @@ auto TestKindCoverage(const std::string& manifest_path,
<< llvm::join(missing_kinds, Bullet);
llvm::SmallVector<std::string> unexpected_matches;
covered_kinds.ForEach(
[&](const std::string& match) { unexpected_matches.push_back(match); });
for (const std::string& match : covered_kinds.entries()) {
unexpected_matches.push_back(match);
}
llvm::sort(unexpected_matches);
EXPECT_TRUE(unexpected_matches.empty())
<< "Matched things that aren't in the kind list:" << Bullet