Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++] Add test coverage for unordered containers comparison #66692

Merged
merged 1 commit into from
Sep 21, 2023

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