* [PATCH] Add some parallel_for_each tests
@ 2021-08-28 19:18 Tom Tromey
2021-08-29 19:58 ` Simon Marchi
0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2021-08-28 19:18 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This adds a few tests of parallel_for_each, primarily testing that
different settings for the number of threads will work. This helps
address an issue that Tom de Vries pointed out in the DWARF scanner
rewrite series -- that a change to parallel_for_each introduced a
regression here.
---
gdb/Makefile.in | 1 +
gdb/unittests/parallel-for-selftests.c | 86 ++++++++++++++++++++++++++
2 files changed, 87 insertions(+)
create mode 100644 gdb/unittests/parallel-for-selftests.c
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 73a1bf83c85..320d3326a81 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -456,6 +456,7 @@ SELFTESTS_SRCS = \
unittests/offset-type-selftests.c \
unittests/observable-selftests.c \
unittests/optional-selftests.c \
+ unittests/parallel-for-selftests.c \
unittests/parse-connection-spec-selftests.c \
unittests/ptid-selftests.c \
unittests/main-thread-selftests.c \
diff --git a/gdb/unittests/parallel-for-selftests.c b/gdb/unittests/parallel-for-selftests.c
new file mode 100644
index 00000000000..7f61b709fa7
--- /dev/null
+++ b/gdb/unittests/parallel-for-selftests.c
@@ -0,0 +1,86 @@
+/* Self tests for parallel_for_each
+
+ Copyright (C) 2021 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "gdbsupport/selftest.h"
+#include "gdbsupport/parallel-for.h"
+#include "gdbsupport/thread-pool.h"
+
+#if CXX_STD_THREAD
+
+namespace selftests {
+namespace parallel_for {
+
+struct save_restore_n_threads
+{
+ save_restore_n_threads ()
+ : n_threads (gdb::thread_pool::g_thread_pool->thread_count ())
+ {
+ }
+
+ ~save_restore_n_threads ()
+ {
+ gdb::thread_pool::g_thread_pool->set_thread_count (n_threads);
+ }
+
+ int n_threads;
+};
+
+static void
+test (int n_threads)
+{
+ save_restore_n_threads saver;
+ gdb::thread_pool::g_thread_pool->set_thread_count (n_threads);
+
+#define NUMBER 10000
+
+ std::atomic<int> counter = 0;
+ gdb::parallel_for_each (0, NUMBER,
+ [&] (int start, int end)
+ {
+ counter += end - start;
+ });
+
+ SELF_CHECK (counter == NUMBER);
+
+#undef NUMBER
+}
+
+static void
+test_n_threads ()
+{
+ test (0);
+ test (1);
+ test (3);
+}
+
+}
+}
+
+#endif /* CXX_STD_THREAD */
+
+void _initialize_parallel_for_selftests ();
+void
+_initialize_parallel_for_selftests ()
+{
+#ifdef CXX_STD_THREAD
+ selftests::register_test ("parallel_for",
+ selftests::parallel_for::test_n_threads);
+#endif /* CXX_STD_THREAD */
+}
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add some parallel_for_each tests
2021-08-28 19:18 [PATCH] Add some parallel_for_each tests Tom Tromey
@ 2021-08-29 19:58 ` Simon Marchi
2021-08-30 2:24 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2021-08-29 19:58 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2021-08-28 3:18 p.m., Tom Tromey wrote:
> This adds a few tests of parallel_for_each, primarily testing that
> different settings for the number of threads will work. This helps
> address an issue that Tom de Vries pointed out in the DWARF scanner
> rewrite series -- that a change to parallel_for_each introduced a
> regression here.
Can you say a few words on what was the bug? I'm reading the test, and
since there are no comments it's unclear what behavior you are testing
specifically (other than "parallel_for_each" in general).
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add some parallel_for_each tests
2021-08-29 19:58 ` Simon Marchi
@ 2021-08-30 2:24 ` Tom Tromey
2021-08-30 13:29 ` Simon Marchi
0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2021-08-30 2:24 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>> This adds a few tests of parallel_for_each, primarily testing that
>> different settings for the number of threads will work. This helps
>> address an issue that Tom de Vries pointed out in the DWARF scanner
>> rewrite series -- that a change to parallel_for_each introduced a
>> regression here.
Simon> Can you say a few words on what was the bug? I'm reading the test, and
Simon> since there are no comments it's unclear what behavior you are testing
Simon> specifically (other than "parallel_for_each" in general).
Do you mean generally, or in the comments in the commit message?
Anyway, the motivation for the patch is to prevent the bug that Tom de
Vries noticed in the DWARF scanner series -- that a change to
parallel_for_each introduced a bug when n_threads==0.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add some parallel_for_each tests
2021-08-30 2:24 ` Tom Tromey
@ 2021-08-30 13:29 ` Simon Marchi
2021-08-30 13:44 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2021-08-30 13:29 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2021-08-29 10:24 p.m., Tom Tromey wrote:
>>> This adds a few tests of parallel_for_each, primarily testing that
>>> different settings for the number of threads will work. This helps
>>> address an issue that Tom de Vries pointed out in the DWARF scanner
>>> rewrite series -- that a change to parallel_for_each introduced a
>>> regression here.
>
> Simon> Can you say a few words on what was the bug? I'm reading the test, and
> Simon> since there are no comments it's unclear what behavior you are testing
> Simon> specifically (other than "parallel_for_each" in general).
>
> Do you mean generally, or in the comments in the commit message?
>
> Anyway, the motivation for the patch is to prevent the bug that Tom de
> Vries noticed in the DWARF scanner series -- that a change to
> parallel_for_each introduced a bug when n_threads==0.
I think you could mention in the commit message that the bug in question
happened when n_threads == 0, and what is was (a crash, hang, bad
result, etc).
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add some parallel_for_each tests
2021-08-30 13:29 ` Simon Marchi
@ 2021-08-30 13:44 ` Tom Tromey
2021-08-30 15:45 ` Simon Marchi
0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2021-08-30 13:44 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
Simon> I think you could mention in the commit message that the bug in question
Simon> happened when n_threads == 0, and what is was (a crash, hang, bad
Simon> result, etc).
Ok, I did that. I'm going to check it in.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add some parallel_for_each tests
2021-08-30 13:44 ` Tom Tromey
@ 2021-08-30 15:45 ` Simon Marchi
0 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2021-08-30 15:45 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2021-08-30 9:44 a.m., Tom Tromey wrote:
> Simon> I think you could mention in the commit message that the bug in question
> Simon> happened when n_threads == 0, and what is was (a crash, hang, bad
> Simon> result, etc).
>
> Ok, I did that. I'm going to check it in.
On Ubuntu 20.04, when compiled with g++-9 or g++-10 (from the g++-9 and
g++-10 repo packages):
CXX unittests/parallel-for-selftests.o
/home/smarchi/src/binutils-gdb/gdb/unittests/parallel-for-selftests.c: In function ‘void selftests::parallel_for::test(int)’:
/home/smarchi/src/binutils-gdb/gdb/unittests/parallel-for-selftests.c:53:30: error: use of deleted function ‘std::atomic<int>::atomic(const std::atomic<int>&)’
53 | std::atomic<int> counter = 0;
| ^
In file included from /usr/include/c++/9/future:42,
from /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/thread-pool.h:29,
from /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/parallel-for.h:26,
from /home/smarchi/src/binutils-gdb/gdb/unittests/parallel-for-selftests.c:22:
/usr/include/c++/9/atomic:755:7: note: declared here
755 | atomic(const atomic&) = delete;
| ^~~~~~
/usr/include/c++/9/atomic:759:17: note: after user-defined conversion: ‘constexpr std::atomic<int>::atomic(std::atomic<int>::__integral_type)’
759 | constexpr atomic(__integral_type __i) noexcept : __base_type(__i) { }
| ^~~~~~
I didn't dig to understand the why, but this works:
std::atomic<int> counter { 0 };
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-30 15:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-28 19:18 [PATCH] Add some parallel_for_each tests Tom Tromey
2021-08-29 19:58 ` Simon Marchi
2021-08-30 2:24 ` Tom Tromey
2021-08-30 13:29 ` Simon Marchi
2021-08-30 13:44 ` Tom Tromey
2021-08-30 15:45 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).