From d8efa09fc28d518ddfc791854377f3bbca319d32 Mon Sep 17 00:00:00 2001 From: David Allemang Date: Sat, 30 Oct 2021 18:55:06 -0400 Subject: [PATCH] Simplify RelTables Structure Some performance hit, but remember that std::vector already does block allocation so it's not too bad. - Remove block-allocation from RelTables - Use std::shared_ptr for lst_ptrs - Replace vector-of-struct-of-vector with vector-of-struct. - Remove rels from RelTables --- include/tc/core.hpp | 2 +- src/core.cpp | 8 +- src/solve.cpp | 174 ++++++++++++++++++++------------------------ 3 files changed, 84 insertions(+), 100 deletions(-) diff --git a/include/tc/core.hpp b/include/tc/core.hpp index 7f91889..a608b75 100644 --- a/include/tc/core.hpp +++ b/include/tc/core.hpp @@ -141,7 +141,7 @@ namespace tc { [[nodiscard]] int get(int a, int b) const; - [[nodiscard]] std::vector rels() const; + [[nodiscard]] std::vector get_rels() const; [[nodiscard]] SubGroup subgroup(const std::vector &gens) const; diff --git a/src/core.cpp b/src/core.cpp index 09ff19d..d9800bc 100644 --- a/src/core.cpp +++ b/src/core.cpp @@ -96,7 +96,7 @@ namespace tc { return _mults[a][b]; } - std::vector Group::rels() const { + std::vector Group::get_rels() const { std::vector res; for (int i = 0; i < ngens - 1; ++i) { for (int j = i + 1; j < ngens; ++j) { @@ -114,9 +114,9 @@ namespace tc { std::stringstream ss; ss << name << "*" << other.name; - Group g(ngens + other.ngens, rels(), ss.str()); + Group g(ngens + other.ngens, get_rels(), ss.str()); - for (const auto &rel : other.rels()) { + for (const auto &rel : other.get_rels()) { g.set(rel.shift(ngens)); } @@ -128,7 +128,7 @@ namespace tc { ss << name << "^" << p; Group g(ngens * p, {}, ss.str()); - for (const auto &rel : rels()) { + for (const auto &rel : get_rels()) { for (int off = 0; off < g.ngens; off += ngens) { g.set(rel.shift(off)); } diff --git a/src/solve.cpp b/src/solve.cpp index 0c144f0..d9988e8 100644 --- a/src/solve.cpp +++ b/src/solve.cpp @@ -1,66 +1,37 @@ #include "tc/core.hpp" #include +#include namespace tc { - struct RelTablesRow { - int *gnrs; - int **lst_ptrs; - - RelTablesRow(int N, int *gnrs, int **lst_ptrs) : gnrs(gnrs), lst_ptrs(lst_ptrs) { - for (int i = 0; i < N; i++) { - lst_ptrs[i] = nullptr; - } - } + struct Row { + int gnr; + std::shared_ptr lst; }; struct RelTables { - static const int ROW_BLOCK_SIZE = 64; - std::vector rels; - std::vector rows; - int start = 0; - int num_tables; - int buffer_rows = 0; + private: + std::vector rows; + size_t nrels; - explicit RelTables(const std::vector &rels) - : num_tables(rels.size()), rels(rels) { + public: + explicit RelTables(size_t nrels) : nrels(nrels) { + } + + Row &operator()(size_t irel, size_t coset) { + // todo check bounds + size_t idx = coset * nrels + irel; + return rows[idx]; } void add_row() { - if (buffer_rows == 0) { - int *gnrs_alloc = new int[num_tables * RelTables::ROW_BLOCK_SIZE]; - int **lst_ptrs_alloc = new int *[num_tables * RelTables::ROW_BLOCK_SIZE]; - for (int i = 0; i < RelTables::ROW_BLOCK_SIZE; i++) { - rows.push_back( - new RelTablesRow(num_tables, &gnrs_alloc[i * num_tables], &lst_ptrs_alloc[i * num_tables])); - } - buffer_rows = RelTables::ROW_BLOCK_SIZE; - } - - buffer_rows--; + // std::vector already does block allocation. + rows.resize(rows.size() + nrels); } - void del_rows_to(int idx) { - const int del_to = (idx / RelTables::ROW_BLOCK_SIZE) * RelTables::ROW_BLOCK_SIZE; - for (int i = start; i < del_to; i += RelTables::ROW_BLOCK_SIZE) { - delete[] rows[i]->gnrs; - delete[] rows[i]->lst_ptrs; - for (int j = 0; j < RelTables::ROW_BLOCK_SIZE; j++) { - delete rows[i + j]; - } - start += RelTables::ROW_BLOCK_SIZE; - } - } - - ~RelTables() { - while (start < rows.size()) { - delete[] rows[start]->gnrs; - delete[] rows[start]->lst_ptrs; - for (int j = 0; j < RelTables::ROW_BLOCK_SIZE; j++) { - delete rows[start + j]; - } - start += RelTables::ROW_BLOCK_SIZE; - } + void del_rows_to(size_t coset) { + /// strictly refers to freeing pre-allocated blocks of for *gnrs and **lst_ptrs. + /// actual `rows` is unchanged. } }; @@ -72,32 +43,39 @@ namespace tc { return cosets; } - for (int g : sub_gens) { + for (int g: sub_gens) { if (g < ngens) cosets.put(0, g, 0); } - RelTables rel_tables(rels()); + + const auto &rels = get_rels(); // todo move to Group member + const auto nrels = rels.size(); + + // todo encapsulate std::vector> gen_map(ngens); int rel_idx = 0; - for (Rel m : rels()) { + for (Rel m: rels) { gen_map[m.gens[0]].push_back(rel_idx); gen_map[m.gens[1]].push_back(rel_idx); rel_idx++; } - int null_lst_ptr; - rel_tables.add_row(); - RelTablesRow &row = *(rel_tables.rows[0]); - for (int table_idx = 0; table_idx < rel_tables.num_tables; table_idx++) { - Rel &ti = rel_tables.rels[table_idx]; + std::shared_ptr null_lst_ptr = std::make_shared(); - if (cosets.get(ti.gens[0]) + cosets.get(ti.gens[1]) == -2) { - row.lst_ptrs[table_idx] = new int; - row.gnrs[table_idx] = 0; + RelTables tables(nrels); + tables.add_row(); + + for (int irel = 0; irel < nrels; irel++) { + Row &row = tables(irel, 0); + const Rel &rel = rels[irel]; + + if (cosets.get(rel.gens[0]) == -1 && cosets.get(rel.gens[1]) == -1) { + row.lst = std::make_shared(); + row.gnr = 0; } else { - row.lst_ptrs[table_idx] = &null_lst_ptr; - row.gnrs[table_idx] = -1; + row.lst = null_lst_ptr; + row.gnr = -1; } } @@ -108,13 +86,13 @@ namespace tc { idx++; if (idx == cosets.data.size()) { - rel_tables.del_rows_to(idx / ngens); + tables.del_rows_to(idx / ngens); break; } target = cosets.size(); cosets.add_row(); - rel_tables.add_row(); + tables.add_row(); std::vector facts; facts.push_back(idx); @@ -122,9 +100,8 @@ namespace tc { coset = idx / ngens; gen = idx % ngens; - rel_tables.del_rows_to(coset); + tables.del_rows_to(coset); - RelTablesRow &target_row = *(rel_tables.rows[target]); while (!facts.empty()) { fact_idx = facts.back(); facts.pop_back(); @@ -137,31 +114,36 @@ namespace tc { coset = fact_idx / ngens; gen = fact_idx % ngens; - if (target == coset) - for (int table_idx : gen_map[gen]) - if (target_row.lst_ptrs[table_idx] == nullptr) - target_row.gnrs[table_idx] = -1; + if (target == coset) { + for (int irel: gen_map[gen]) { + Row &target_row = tables(irel, target); + if (target_row.lst == nullptr) + target_row.gnr = -1; + } + } - RelTablesRow &coset_row = *(rel_tables.rows[coset]); - for (int table_idx : gen_map[gen]) { - if (target_row.lst_ptrs[table_idx] == nullptr) { - Rel &ti = rel_tables.rels[table_idx]; - target_row.lst_ptrs[table_idx] = coset_row.lst_ptrs[table_idx]; - target_row.gnrs[table_idx] = coset_row.gnrs[table_idx] + 1; + for (int irel: gen_map[gen]) { + Row &target_row = tables(irel, target); + Row &coset_row = tables(irel, coset); - if (coset_row.gnrs[table_idx] < 0) - target_row.gnrs[table_idx] -= 2; + if (target_row.lst == nullptr) { + const Rel &rel = rels[irel]; + target_row.lst = coset_row.lst; + target_row.gnr = coset_row.gnr + 1; - if (target_row.gnrs[table_idx] == ti.mult) { - lst = *(target_row.lst_ptrs[table_idx]); - delete target_row.lst_ptrs[table_idx]; - gen_ = ti.gens[(int) (ti.gens[0] == gen)]; + if (coset_row.gnr < 0) { + target_row.gnr -= 2; + } + + if (target_row.gnr == rel.mult) { + lst = *target_row.lst; + gen_ = rel.gens[rel.gens[0] == gen]; facts.push_back(lst * ngens + gen_); - } else if (target_row.gnrs[table_idx] == -ti.mult) { - gen_ = ti.gens[ti.gens[0] == gen]; + } else if (target_row.gnr == -rel.mult) { + gen_ = rel.gens[rel.gens[0] == gen]; facts.push_back(target * ngens + gen_); - } else if (target_row.gnrs[table_idx] == ti.mult - 1) { - *(target_row.lst_ptrs[table_idx]) = target; + } else if (target_row.gnr == rel.mult - 1) { + *target_row.lst = target; } } } @@ -169,16 +151,18 @@ namespace tc { std::sort(facts.begin(), facts.end(), std::greater<>()); } - for (int table_idx = 0; table_idx < rel_tables.num_tables; table_idx++) { - Rel &ti = rel_tables.rels[table_idx]; - if (target_row.lst_ptrs[table_idx] == nullptr) { - if ((cosets.get(target, ti.gens[0]) != target) and - (cosets.get(target, ti.gens[1]) != target)) { - target_row.lst_ptrs[table_idx] = new int; - target_row.gnrs[table_idx] = 0; + for (int irel = 0; irel < nrels; irel++) { + Row &target_row = tables(irel, target); + const Rel &rel = rels[irel]; + + if (target_row.lst == nullptr) { + if ((cosets.get(target, rel.gens[0]) != target) and + (cosets.get(target, rel.gens[1]) != target)) { + target_row.lst = std::make_shared(); + target_row.gnr = 0; } else { - target_row.lst_ptrs[table_idx] = &null_lst_ptr; - target_row.gnrs[table_idx] = -1; + target_row.lst = null_lst_ptr; + target_row.gnr = -1; } } }