[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Parallelize test read-dwarf
Ok, V3 coming up ;)
On 07.10.2015 10:43, Dodji Seketeli wrote:
Hey,
Ondrej Oprala <ooprala@redhat.com> a Ãcrit:
Thanks for the late-night review!
It's my pleasure, thank you for the patch!
[...]
+int
+main(int argc, char *argv[])
+{
+ // Number of currently online processors in the system.
+ size_t nprocs = sysconf(_SC_NPROCESSORS_ONLN);
Ah, why not detecting the --no-parallel before this point, and if it's
set to true, then just set nprocs to 1? And just keep the same neat
logic you have with the threading.
+ // All the pthread_ts we've created.
+ pthread_t *pthr = new pthread_t[nprocs];
Before this point, I'd assert that nprocs is at least 1.
Cheers,
Thx,
Ondrej
>From 6e9fbb82a85aed6c2c3df44c74d6ddcef8878773 Mon Sep 17 00:00:00 2001
From: Ondrej Oprala <ooprala@redhat.com>
Date: Tue, 6 Oct 2015 14:35:14 +0200
Subject: [PATCH] Parallelize test read-dwarf.
* tests/Makefile.am: Link runtestreaddwarf with libpthread.
* tests/test-read-dwarf.cc (main) Create worker threads corresponding
to the number of CPUs online, add a "--no-parallel" option and move
the main loop...
(handleInOutSpec) ...here.
Signed-off-by: Ondrej Oprala <ooprala@redhat.com>
---
tests/Makefile.am | 1 +
tests/test-read-dwarf.cc | 209 +++++++++++++++++++++++++++++------------------
2 files changed, 131 insertions(+), 79 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8ab7bb6..cf8f2af 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -55,6 +55,7 @@ runtestwritereadarchive_LDADD= libtestutils.la $(top_builddir)/src/libabigail.la
runtestreaddwarf_SOURCES=test-read-dwarf.cc
runtestreaddwarf_LDADD=libtestutils.la $(top_builddir)/src/libabigail.la
+runtestreaddwarf_LDFLAGS=-pthread
runtestlookupsyms_SOURCES=test-lookup-syms.cc
runtestlookupsyms_LDADD=libtestutils.la $(top_builddir)/src/libabigail.la
diff --git a/tests/test-read-dwarf.cc b/tests/test-read-dwarf.cc
index c4fed5f..d534c85 100644
--- a/tests/test-read-dwarf.cc
+++ b/tests/test-read-dwarf.cc
@@ -28,6 +28,8 @@
#include <fstream>
#include <iostream>
#include <cstdlib>
+#include <unistd.h>
+#include <pthread.h>
#include "abg-ir.h"
#include "abg-dwarf-reader.h"
#include "abg-writer.h"
@@ -38,6 +40,7 @@ using std::string;
using std::ofstream;
using std::cerr;
using abigail::tests::get_build_dir;
+using abigail::dwarf_reader::read_corpus_from_elf;
/// This is an aggregate that specifies where a test shall get its
/// input from, and where it shall write its ouput to.
@@ -150,96 +153,144 @@ InOutSpec in_out_specs[] =
{NULL, NULL, NULL}
};
-int
-main()
-{
- using abigail::dwarf_reader::read_corpus_from_elf;
- unsigned result = 1;
+// The global pointer to the testsuite paths.
+InOutSpec *iospec = in_out_specs;
+// Lock to help atomically increment iospec.
+pthread_mutex_t spec_lock = PTHREAD_MUTEX_INITIALIZER;
+pthread_mutex_t write_lock = PTHREAD_MUTEX_INITIALIZER;
+// No lock needed here, since is_ok is only ever re-set to false.
+bool is_ok = true;
+bool no_parallel;
- bool is_ok = true;
+// These prefixes don't change during the program's lifetime, so
+// we only get them once.
+const string out_abi_base = get_build_dir() + "/tests/";
+const string in_elf_base = abigail::tests::get_src_dir() + "/tests/";
+const string in_abi_base = in_elf_base;
+
+void
+handle_in_out_spec(void)
+{
string in_elf_path, in_abi_path, out_abi_path;
abigail::ir::environment_sptr env;
- abigail::corpus_sptr corp;
+ InOutSpec *s;
- for (InOutSpec* s = in_out_specs; s->in_elf_path; ++s)
- {
- in_elf_path = abigail::tests::get_src_dir() + "/tests/" + s->in_elf_path;
- env.reset(new abigail::ir::environment);
- abigail::dwarf_reader::status status =
- abigail::dwarf_reader::STATUS_UNKNOWN;
- corp = read_corpus_from_elf(in_elf_path,
- /*debug_info_root_path=*/0,
- env.get(),
- /*load_all_types=*/false,
- status);
- if (!corp)
- {
- cerr << "failed to read " << in_elf_path << "\n";
- is_ok = false;
- continue;
- }
- corp->set_path(s->in_elf_path);
- // Do not take architecture names in comparison so that these
- // test input binaries can come from whatever arch the
- // programmer likes.
- corp->set_architecture_name("");
-
- out_abi_path = get_build_dir() + "/tests/" + s->out_abi_path;
- if (!abigail::tools_utils::ensure_parent_dir_created(out_abi_path))
- {
- cerr << "Could not create parent director for " << out_abi_path;
- is_ok = false;
- return result;
- }
+ while (true)
+ {
+ pthread_mutex_lock(&spec_lock);
+ if (iospec->in_elf_path)
+ s = iospec++;
+ else
+ s = NULL;
+ pthread_mutex_unlock(&spec_lock);
- ofstream of(out_abi_path.c_str(), std::ios_base::trunc);
- if (!of.is_open())
- {
- cerr << "failed to read " << out_abi_path << "\n";
- is_ok = false;
- continue;
- }
+ if (!s)
+ pthread_exit(NULL);
+ in_elf_path = in_elf_base + s->in_elf_path;
+ env.reset(new abigail::ir::environment);
+ abigail::dwarf_reader::status status =
+ abigail::dwarf_reader::STATUS_UNKNOWN;
+ abigail::corpus_sptr corp =
+ read_corpus_from_elf(in_elf_path,
+ /*debug_info_root_path=*/0,
+ env.get(),
+ /*load_all_types=*/false,
+ status);
+ if (!corp)
+ {
+ cerr << "failed to read " << in_elf_path << "\n";
+ is_ok = false;
+ continue;
+ }
+ corp->set_path(s->in_elf_path);
+ // Do not take architecture names in comparison so that these
+ // test input binaries can come from whatever arch the
+ // programmer likes.
+ corp->set_architecture_name("");
- bool r =
- abigail::xml_writer::write_corpus_to_native_xml(corp,
- /*indent=*/0,
- of);
- is_ok = (is_ok && r);
- of.close();
-
- string abilint = get_build_dir() + "/tools/abilint";
- abilint += " --noout";
- string cmd = abilint + " " + out_abi_path;
- if (system(cmd.c_str()))
- {
- cerr << "output file doesn't pass abilint: " << out_abi_path << "\n";
- r = false;
- }
- is_ok = (is_ok && r);
+ out_abi_path = out_abi_base + s->out_abi_path;
+ if (!abigail::tools_utils::ensure_parent_dir_created(out_abi_path))
+ {
+ cerr << "Could not create parent directory for " << out_abi_path;
+ is_ok = false;
+ exit(1);
+ }
+ ofstream of(out_abi_path.c_str(), std::ios_base::trunc);
+ if (!of.is_open())
+ {
+ cerr << "failed to read " << out_abi_path << "\n";
+ is_ok = false;
+ continue;
+ }
- in_elf_path = abigail::tests::get_src_dir() + "/tests/" + s->in_elf_path;
- string abidiff = get_build_dir() + "/tools/abidiff";
- cmd = abidiff + " --no-architecture " + in_elf_path + " " + out_abi_path;
- r = true;
- if (system(cmd.c_str()))
+ pthread_mutex_lock(&write_lock);
+ is_ok =
+ abigail::xml_writer::write_corpus_to_native_xml(corp,
+ /*indent=*/0,
+ of);
+ pthread_mutex_unlock(&write_lock);
+ of.close();
+
+ string abilint = get_build_dir() + "/tools/abilint";
+ abilint += " --noout";
+ string cmd = abilint + " " + out_abi_path;
+ if (system(cmd.c_str()))
+ {
+ cerr << "output file doesn't pass abilint: " << out_abi_path << "\n";
+ is_ok = false;
+ }
+
+ string abidiff = get_build_dir() + "/tools/abidiff";
+ cmd = abidiff + " --no-architecture " + in_elf_path + " " + out_abi_path;
+ if (system(cmd.c_str()))
+ {
+ cerr << "ABIs differ:\n"
+ << in_elf_path
+ << "\nand:\n"
+ << out_abi_path
+ << "\n";
+ is_ok = false;
+ }
+
+ in_abi_path = in_abi_base + s->in_abi_path;
+ cmd = "diff -u " + in_abi_path + " " + out_abi_path;
+ if (system(cmd.c_str()))
+ is_ok = false;
+ }
+}
+
+int
+main(int argc, char *argv[])
+{
+ // Number of currently online processors in the system.
+ size_t nprocs = sysconf(_SC_NPROCESSORS_ONLN);
+ // All the pthread_ts we've created.
+ pthread_t *pthr = new pthread_t[nprocs];
+
+ if (argc == 2)
+ {
+ if (argv[1] == string("--no-parallel"))
+ nprocs = 1;
+ else
{
- cerr << "ABIs differ:\n"
- << in_elf_path
- << "\nand:\n"
- << out_abi_path
- << "\n";
- r = false;
+ cerr << "unrecognized option\n";
+ cerr << "usage: " << argv[0] << " [--no-parallel]\n" ;
+ return 1;
}
- is_ok = (is_ok && r);
-
- in_abi_path = abigail::tests::get_src_dir() + "/tests/" + s->in_abi_path;
- cmd = "diff -u " + in_abi_path + " " + out_abi_path;
- r = true;
- if (system(cmd.c_str()))
- r = false;
- is_ok = (is_ok && r);
}
+ assert(nprocs >= 1);
+
+ for (size_t i = 0; i < nprocs; ++i)
+ pthread_create(&pthr[i], NULL,
+ (void*(*)(void*))handle_in_out_spec,
+ NULL);
+
+ for (size_t i = 0; i < nprocs; ++i)
+ pthread_join(pthr[i], NULL);
+
+ delete pthr;
+
return !is_ok;
}
--
2.4.3