Skip to content

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Sep 18, 2023

This patch is a melting pot of changes picked up from https://llvm.org/D61878. It adds a few tests checking corner cases of unordered containers comparison and adds benchmarks for a few unordered_set operations.

@ldionne ldionne requested a review from a team as a code owner September 18, 2023 19:52
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 18, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 18, 2023

@llvm/pr-subscribers-libcxx

Changes

This patch is a melting pot of changes picked up from https://llvm.org/D61878. It adds a few tests checking corner cases of unordered containers comparison and removes an unnecessary check of std::is_permutation inside unordered_multiset's operator==.


Full diff: https://github.com/llvm/llvm-project/pull/66692.diff

5 Files Affected:

  • (modified) libcxx/benchmarks/ContainerBenchmarks.h (+31)
  • (modified) libcxx/benchmarks/unordered_set_operations.bench.cpp (+19)
  • (modified) libcxx/include/unordered_set (+1-4)
  • (modified) libcxx/test/std/containers/unord/unord.multimap/eq.pass.cpp (+26-1)
  • (modified) libcxx/test/std/containers/unord/unord.multiset/eq.pass.cpp (+14-1)
diff --git a/libcxx/benchmarks/ContainerBenchmarks.h b/libcxx/benchmarks/ContainerBenchmarks.h
index 5ab6f619b85a8d0..071e46c2a1c6546 100644
--- a/libcxx/benchmarks/ContainerBenchmarks.h
+++ b/libcxx/benchmarks/ContainerBenchmarks.h
@@ -179,6 +179,37 @@ static void BM_Rehash(benchmark::State& st, Container c, GenInputs gen) {
   }
 }
 
+template <class Container, class GenInputs>
+static void BM_Compare_same_container(benchmark::State& st, Container, GenInputs gen) {
+  auto in = gen(st.range(0));
+  Container c1(in.begin(), in.end());
+  Container c2 = c1;
+
+  benchmark::DoNotOptimize(&(*c1.begin()));
+  benchmark::DoNotOptimize(&(*c2.begin()));
+  while (st.KeepRunning()) {
+    bool res = c1 == c2;
+    benchmark::DoNotOptimize(&res);
+    benchmark::ClobberMemory();
+  }
+}
+
+template <class Container, class GenInputs>
+static void BM_Compare_different_containers(benchmark::State& st, Container, GenInputs gen) {
+  auto in1 = gen(st.range(0));
+  auto in2 = gen(st.range(0));
+  Container c1(in1.begin(), in1.end());
+  Container c2(in2.begin(), in2.end());
+
+  benchmark::DoNotOptimize(&(*c1.begin()));
+  benchmark::DoNotOptimize(&(*c2.begin()));
+  while (st.KeepRunning()) {
+    bool res = c1 == c2;
+    benchmark::DoNotOptimize(&res);
+    benchmark::ClobberMemory();
+  }
+}
+
 } // end namespace ContainerBenchmarks
 
 #endif // BENCHMARK_CONTAINER_BENCHMARKS_H
diff --git a/libcxx/benchmarks/unordered_set_operations.bench.cpp b/libcxx/benchmarks/unordered_set_operations.bench.cpp
index 96aa2e0dfea3be0..d49de576ec9778d 100644
--- a/libcxx/benchmarks/unordered_set_operations.bench.cpp
+++ b/libcxx/benchmarks/unordered_set_operations.bench.cpp
@@ -290,6 +290,25 @@ BENCHMARK_CAPTURE(BM_Rehash,
 BENCHMARK_CAPTURE(BM_Rehash, unordered_set_int_arg, std::unordered_set<int>{}, getRandomIntegerInputs<int>)
     ->Arg(TestNumInputs);
 
+//----------------------------------------------------------------------------//
+//                         BM_Compare
+// ---------------------------------------------------------------------------//
+
+BENCHMARK_CAPTURE(
+    BM_Compare_same_container, unordered_set_string, std::unordered_set<std::string>{}, getRandomStringInputs)
+    ->Arg(TestNumInputs);
+
+BENCHMARK_CAPTURE(BM_Compare_same_container, unordered_set_int, std::unordered_set<int>{}, getRandomIntegerInputs<int>)
+    ->Arg(TestNumInputs);
+
+BENCHMARK_CAPTURE(
+    BM_Compare_different_containers, unordered_set_string, std::unordered_set<std::string>{}, getRandomStringInputs)
+    ->Arg(TestNumInputs);
+
+BENCHMARK_CAPTURE(
+    BM_Compare_different_containers, unordered_set_int, std::unordered_set<int>{}, getRandomIntegerInputs<int>)
+    ->Arg(TestNumInputs);
+
 ///////////////////////////////////////////////////////////////////////////////
 BENCHMARK_CAPTURE(BM_InsertDuplicate, unordered_set_int, std::unordered_set<int>{}, getRandomIntegerInputs<int>)
     ->Arg(TestNumInputs);
diff --git a/libcxx/include/unordered_set b/libcxx/include/unordered_set
index 5e47f12446ff936..6442bef93114956 100644
--- a/libcxx/include/unordered_set
+++ b/libcxx/include/unordered_set
@@ -527,7 +527,6 @@ template <class Value, class Hash, class Pred, class Alloc>
 
 */
 
-#include <__algorithm/is_permutation.h>
 #include <__assert> // all public C++ headers provide the assertion handler
 #include <__availability>
 #include <__config>
@@ -1935,9 +1934,7 @@ operator==(const unordered_multiset<_Value, _Hash, _Pred, _Alloc>& __x,
     {
         _EqRng __xeq = __x.equal_range(*__i);
         _EqRng __yeq = __y.equal_range(*__i);
-        if (_VSTD::distance(__xeq.first, __xeq.second) !=
-            _VSTD::distance(__yeq.first, __yeq.second) ||
-                  !_VSTD::is_permutation(__xeq.first, __xeq.second, __yeq.first))
+        if (std::distance(__xeq.first, __xeq.second) != std::distance(__yeq.first, __yeq.second))
             return false;
         __i = __xeq.second;
     }
diff --git a/libcxx/test/std/containers/unord/unord.multimap/eq.pass.cpp b/libcxx/test/std/containers/unord/unord.multimap/eq.pass.cpp
index d0e578e190c0a18..2258e31d75b44e2 100644
--- a/libcxx/test/std/containers/unord/unord.multimap/eq.pass.cpp
+++ b/libcxx/test/std/containers/unord/unord.multimap/eq.pass.cpp
@@ -221,5 +221,30 @@ int main(int, char**)
     }
 #endif
 
-  return 0;
+    // Make sure we take into account the number of times that a key repeats into equality.
+    {
+        typedef std::pair<int, char> P;
+        P a[] = {{1, 'a'}, {1, 'b'},           {1, 'd'}, {2, 'b'}};
+        P b[] = {{1, 'a'}, {1, 'b'}, {1, 'b'}, {1, 'd'}, {2, 'b'}};
+
+        std::unordered_multimap<int, char> c1(std::begin(a), std::end(a));
+        std::unordered_multimap<int, char> c2(std::begin(b), std::end(b));
+        assert(testEquality(c1, c2, false));
+    }
+
+    // Make sure we incorporate the values into the equality of the maps.
+    // If we were to compare only the keys (including how many time each key repeats),
+    // the following test would fail cause only the values differ.
+    {
+        typedef std::pair<int, char> P;
+        P a[] = {{1, 'a'}, {1, 'b'}, {1, 'd'}, {2, 'b'}};
+        P b[] = {{1, 'a'}, {1, 'b'}, {1, 'E'}, {2, 'b'}};
+        //                                ^ different here
+
+        std::unordered_multimap<int, char> c1(std::begin(a), std::end(a));
+        std::unordered_multimap<int, char> c2(std::begin(b), std::end(b));
+        assert(testEquality(c1, c2, false));
+    }
+
+    return 0;
 }
diff --git a/libcxx/test/std/containers/unord/unord.multiset/eq.pass.cpp b/libcxx/test/std/containers/unord/unord.multiset/eq.pass.cpp
index 22d3207d0ff74a0..c059b461a830789 100644
--- a/libcxx/test/std/containers/unord/unord.multiset/eq.pass.cpp
+++ b/libcxx/test/std/containers/unord/unord.multiset/eq.pass.cpp
@@ -24,6 +24,8 @@
 #include "test_macros.h"
 #include "min_allocator.h"
 
+#include "test_comparisons.h"
+
 int main(int, char**)
 {
     {
@@ -178,5 +180,16 @@ int main(int, char**)
     }
 #endif
 
-  return 0;
+    // Make sure we take into account the number of times that a key repeats into equality.
+    {
+        typedef int P;
+        P a[] = {P(1), P(1), P(1), P(2)};
+        P b[] = {P(1), P(1), P(1), P(1), P(2)};
+
+        std::unordered_multiset<int> c1(std::begin(a), std::end(a));
+        std::unordered_multiset<int> c2(std::begin(b), std::end(b));
+        assert(testEquality(c1, c2, false));
+    }
+
+    return 0;
 }

@ldionne ldionne force-pushed the review/unordered-containers-tests branch from c49c73d to eec0fbd Compare September 18, 2023 22:34
@ldionne ldionne changed the title [libc++] Minor change to how unordered_set is compared [libc++] Add test coverage for unordered containers comparison Sep 18, 2023
This patch is a melting pot of changes picked up from https://llvm.org/D61878.
It adds a few tests checking corner cases of unordered containers comparison
and adds benchmarks for a few unordered_set operations.
@ldionne ldionne force-pushed the review/unordered-containers-tests branch from eec0fbd to d0b99a2 Compare September 19, 2023 15:57
@ldionne ldionne merged commit 8bbcc6d into llvm:main Sep 21, 2023
@ldionne ldionne deleted the review/unordered-containers-tests branch September 21, 2023 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants