From fc729584e15504608eb3b6409292705d42685ee0 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Fri, 13 Sep 2024 23:28:16 +0200 Subject: [PATCH 1/6] Maybe std::deque::size was the problem --- src/db/db/dbPolygonGenerators.cc | 57 ++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/src/db/db/dbPolygonGenerators.cc b/src/db/db/dbPolygonGenerators.cc index ec72e55b5..5b8c2d107 100644 --- a/src/db/db/dbPolygonGenerators.cc +++ b/src/db/db/dbPolygonGenerators.cc @@ -56,13 +56,13 @@ public: typedef contour_type::iterator iterator; PGPolyContour () - : m_is_hole (false), m_next (-1), m_last (-1) + : m_is_hole (false), m_next (-1), m_last (-1), m_size (0) { // ... } PGPolyContour (const PGPolyContour &d) - : m_contour (d.m_contour), m_is_hole (d.m_is_hole), m_next (d.m_next), m_last (d.m_last) + : m_contour (d.m_contour), m_is_hole (d.m_is_hole), m_next (d.m_next), m_last (d.m_last), m_size (d.m_size) { // ... } @@ -74,6 +74,7 @@ public: m_is_hole = d.m_is_hole; m_next = d.m_next; m_last = d.m_last; + m_size = d.m_size; } return *this; } @@ -86,14 +87,45 @@ public: db::Point &front () { return m_contour.front (); } const db::Point &back () const { return m_contour.back (); } db::Point &back () { return m_contour.back (); } - void push_back (const db::Point &p) { m_contour.push_back (p); } - void push_front (const db::Point &p) { m_contour.push_front (p); } - void pop_back () { m_contour.pop_back (); } - void pop_front () { m_contour.pop_front (); } - iterator erase (iterator i) { return m_contour.erase (i); } - iterator insert (iterator i, const db::Point &p) { return m_contour.insert (i, p); } + + void push_back (const db::Point &p) + { + m_contour.push_back (p); + ++m_size; + } + + void push_front (const db::Point &p) + { + m_contour.push_front (p); + ++m_size; + } + + void pop_back () + { + m_contour.pop_back (); + --m_size; + } + + void pop_front () + { + m_contour.pop_front (); + --m_size; + } + + iterator erase (iterator i) + { + --m_size; + return m_contour.erase (i); + } + + iterator insert (iterator i, const db::Point &p) + { + ++m_size; + return m_contour.insert (i, p); + } + bool empty () const { return m_contour.empty (); } - size_t size () const { return m_contour.size (); } + size_t size () const { return m_size; } void last (long n) { @@ -127,19 +159,23 @@ public: void clear () { + m_size = 0; m_next = -1; m_last = -1; m_contour.clear (); } - void erase (iterator from, iterator to) + void erase (iterator from, iterator to) { + m_size -= std::distance (from, to); m_contour.erase (from, to); } template iterator insert (iterator at, I from, I to) { + m_size += std::distance (from, to); + // NOTE: in some STL m_contour.insert already returns the new iterator size_t index_at = at - m_contour.begin (); m_contour.insert (at, from, to); @@ -151,6 +187,7 @@ private: bool m_is_hole; long m_next; long m_last; + size_t m_size; }; From 9e55a664fbf01995111bd9498e307ca4e68ea5e7 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 14 Sep 2024 10:45:28 +0200 Subject: [PATCH 2/6] [debug] timers with ns resolution --- src/db/db/dbPolygonGenerators.cc | 12 +++++++++- src/tl/tl/tlTimer.cc | 38 ++++++++++++++++---------------- src/tl/tl/tlTimer.h | 20 ++++++++--------- 3 files changed, 40 insertions(+), 30 deletions(-) diff --git a/src/db/db/dbPolygonGenerators.cc b/src/db/db/dbPolygonGenerators.cc index 5b8c2d107..158c98905 100644 --- a/src/db/db/dbPolygonGenerators.cc +++ b/src/db/db/dbPolygonGenerators.cc @@ -23,6 +23,7 @@ #include "dbPolygonGenerators.h" +#include "tlTimer.h" #include #include @@ -175,11 +176,14 @@ public: iterator insert (iterator at, I from, I to) { m_size += std::distance (from, to); - +#if 0 // NOTE: in some STL m_contour.insert already returns the new iterator size_t index_at = at - m_contour.begin (); m_contour.insert (at, from, to); return m_contour.begin () + index_at; +#else + return m_contour.insert (at, from, to); +#endif } private: @@ -701,14 +705,20 @@ PolygonGenerator::join_contours (db::Coord x) } else if (! m_open_pos->first && ! n->first) { +{ + tl::SelfTimer timer ("PGContour::insert"); // remove c1 from list of contours, join with c2 if (c2.is_hole ()) { c2.insert (c2.end (), c1.begin () + 1, c1.end ()); } else { c2.insert (c2.begin (), c1.begin (), c1.end () - 1); } +} +{ + tl::SelfTimer timer ("join_contours"); mp_contours->join (i2, i1); +} open_map_iterator_type o = m_open_pos; do { diff --git a/src/tl/tl/tlTimer.cc b/src/tl/tl/tlTimer.cc index f39c5911e..d2fa1ba4c 100644 --- a/src/tl/tl/tlTimer.cc +++ b/src/tl/tl/tlTimer.cc @@ -82,9 +82,9 @@ void current_utc_time (struct timespec *ts) } // ------------------------------------------------------------- -// Gets the current time in ms from epoch +// Gets the current time in ns from epoch -static int64_t ms_time () +static int64_t ns_time () { #if defined(__MACH__) && defined(__APPLE__) @@ -94,7 +94,7 @@ static int64_t ms_time () clock_get_time(cclock, &mts); mach_port_deallocate(mach_task_self(), cclock); - return int64_t (mts.tv_sec) * 1000 + int64_t (0.5 + mts.tv_nsec / 1.0e6); + return int64_t (mts.tv_sec) * 1000000000 + int64_t (mts.tv_nsec); #elif defined(_MSC_VER) @@ -104,14 +104,14 @@ static int64_t ms_time () uint64_t t = (uint64_t (ft.dwHighDateTime) << (sizeof (ft.dwHighDateTime) * 8)) | uint64_t (ft.dwLowDateTime); t -= ft_to_epoch_offset; - // FILETIME uses 100ns resolution, hence divide by 10000 to get ms: - return int64_t (t / 10000); + // FILETIME uses 100ns resolution, hence multiply with 100 to get ns: + return int64_t (t) * 100; #else timespec ts; clock_gettime (CLOCK_REALTIME, &ts); - return int64_t (ts.tv_sec) * 1000 + int64_t (0.5 + ts.tv_nsec / 1.0e6); + return int64_t (ts.tv_sec) * 1000000000 + int64_t (ts.tv_nsec); #endif } @@ -120,8 +120,8 @@ static int64_t ms_time () // Implementation of Timer Timer::Timer () - : m_user_ms (0), m_sys_ms (0), m_wall_ms (0), - m_user_ms_res (0), m_sys_ms_res (0), m_wall_ms_res (0) + : m_user_ms (0), m_sys_ms (0), m_wall_ns (0), + m_user_ms_res (0), m_sys_ms_res (0), m_wall_ns_res (0) { // .. } @@ -142,7 +142,7 @@ Timer::start () m_sys_ms += (timer_t) ((clks.tms_stime + clks.tms_cstime) * clk2msec + 0.5); #endif - m_wall_ms += ms_time (); + m_wall_ns += ns_time (); } void @@ -150,15 +150,15 @@ Timer::stop () { m_user_ms = -m_user_ms; m_sys_ms = -m_sys_ms; - m_wall_ms = -m_wall_ms; + m_wall_ns = -m_wall_ns; start (); m_user_ms_res = m_user_ms; m_sys_ms_res = m_sys_ms; - m_wall_ms_res = m_wall_ms; + m_wall_ns_res = m_wall_ns; m_user_ms = 0; m_sys_ms = 0; - m_wall_ms = 0; + m_wall_ns = 0; } void @@ -166,20 +166,20 @@ Timer::take () { timer_t user_ms = m_user_ms; timer_t sys_ms = m_sys_ms; - timer_t wall_ms = m_wall_ms; + timer_t wall_ns = m_wall_ns; m_user_ms = -m_user_ms; m_sys_ms = -m_sys_ms; - m_wall_ms = -m_wall_ms; + m_wall_ns = -m_wall_ns; start (); m_user_ms_res = m_user_ms; m_sys_ms_res = m_sys_ms; - m_wall_ms_res = m_wall_ms; + m_wall_ns_res = m_wall_ns; m_user_ms = user_ms; m_sys_ms = sys_ms; - m_wall_ms = wall_ms; + m_wall_ns = wall_ns; } size_t @@ -288,20 +288,20 @@ SelfTimer::report () const Clock::Clock (double s) { - m_clock_ms = s * 1000.0; + m_clock_ns = s * 1e9; } double Clock::seconds () const { - return double (m_clock_ms) * 0.001; + return double (m_clock_ns) * 1e-9; } Clock Clock::current () { Clock c; - c.m_clock_ms += ms_time (); + c.m_clock_ns += ns_time (); return c; } diff --git a/src/tl/tl/tlTimer.h b/src/tl/tl/tlTimer.h index 138c8e200..19a7d38a7 100644 --- a/src/tl/tl/tlTimer.h +++ b/src/tl/tl/tlTimer.h @@ -92,7 +92,7 @@ public: */ double sec_wall () const { - return (double (m_wall_ms_res) * 0.001); + return (double (m_wall_ns_res) * 1e-9); } /** @@ -101,8 +101,8 @@ public: static size_t memory_size (); private: - timer_t m_user_ms, m_sys_ms, m_wall_ms; - timer_t m_user_ms_res, m_sys_ms_res, m_wall_ms_res; + timer_t m_user_ms, m_sys_ms, m_wall_ns; + timer_t m_user_ms_res, m_sys_ms_res, m_wall_ns_res; }; /** @@ -182,7 +182,7 @@ public: /** * @brief Default constructor: construct a clock object pointing to an arbitrary value */ - Clock () : m_clock_ms (0) + Clock () : m_clock_ns (0) { // .. nothing yet .. } @@ -196,7 +196,7 @@ public: * @brief Copy constructor */ Clock (const Clock &d) - : m_clock_ms (d.m_clock_ms) + : m_clock_ns (d.m_clock_ns) { // .. nothing yet .. } @@ -206,7 +206,7 @@ public: */ Clock &operator= (Clock d) { - m_clock_ms = d.m_clock_ms; + m_clock_ns = d.m_clock_ns; return *this; } @@ -215,7 +215,7 @@ public: */ bool operator== (Clock d) const { - return m_clock_ms == d.m_clock_ms; + return m_clock_ns == d.m_clock_ns; } /** @@ -231,7 +231,7 @@ public: */ bool operator< (Clock d) const { - return m_clock_ms < d.m_clock_ms; + return m_clock_ns < d.m_clock_ns; } /** @@ -239,7 +239,7 @@ public: */ Clock &operator-= (Clock d) { - m_clock_ms -= d.m_clock_ms; + m_clock_ns -= d.m_clock_ns; return *this; } @@ -264,7 +264,7 @@ public: static Clock current (); private: - timer_t m_clock_ms; + timer_t m_clock_ns; }; } // namespace tl From 79dd2084ceb5e468a7398d216dfec23b2ff2e7f2 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 14 Sep 2024 13:52:25 +0200 Subject: [PATCH 3/6] tl::list enhancement --- src/tl/tl/tlList.h | 297 ++++++++++++++++++++++--------- src/tl/tl/tlTimer.cc | 15 +- src/tl/unit_tests/tlListTests.cc | 67 +++++++ 3 files changed, 287 insertions(+), 92 deletions(-) diff --git a/src/tl/tl/tlList.h b/src/tl/tl/tlList.h index 2d5b572a0..3f5ba29dd 100644 --- a/src/tl/tl/tlList.h +++ b/src/tl/tl/tlList.h @@ -109,16 +109,130 @@ private: bool m_owned; }; +/** + * @brief An iterator for the linked list + */ +template +class list_iterator +{ +public: + typedef std::bidirectional_iterator_tag category; + typedef C value_type; + typedef C &reference; + typedef C *pointer; + + list_iterator (C *p = 0) : mp_p (p) { } + list_iterator operator++ () { mp_p = static_cast (mp_p->mp_next); return *this; } + list_iterator operator-- () { mp_p = static_cast (mp_p->mp_prev); return *this; } + + list_iterator operator++ (int) + { + list_iterator r = *this; + ++*this; + return r; + } + + list_iterator operator-- (int) + { + list_iterator r = *this; + --*this; + return r; + } + + C *operator-> () const + { + return mp_p; + } + + C &operator* () const + { + return *mp_p; + } + + bool operator== (list_iterator other) const { return mp_p == other.mp_p; } + bool operator!= (list_iterator other) const { return mp_p != other.mp_p; } + +private: + C *mp_p; +}; + +/** + * @brief A reverse iterator for the linked list + */ +template +class reverse_list_iterator +{ +public: + typedef std::bidirectional_iterator_tag category; + typedef C value_type; + typedef C &reference; + typedef C *pointer; + + reverse_list_iterator (C *p = 0) : mp_p (p) { } + reverse_list_iterator operator++ () { mp_p = static_cast (mp_p->mp_prev); return *this; } + reverse_list_iterator operator-- () { mp_p = static_cast (mp_p->mp_next); return *this; } + + reverse_list_iterator operator++ (int) + { + reverse_list_iterator r = *this; + ++*this; + return r; + } + + reverse_list_iterator operator-- (int) + { + reverse_list_iterator r = *this; + --*this; + return r; + } + + C *operator-> () const + { + return mp_p; + } + + C &operator* () const + { + return *mp_p; + } + + bool operator== (reverse_list_iterator other) const { return mp_p == other.mp_p; } + bool operator!= (reverse_list_iterator other) const { return mp_p != other.mp_p; } + +private: + C *mp_p; +}; + template class list_impl { public: + typedef list_iterator iterator; + typedef list_iterator const_iterator; + typedef reverse_list_iterator reverse_iterator; + typedef reverse_list_iterator const_reverse_iterator; + + typedef C value_type; + list_impl () : m_head (), m_back () { m_head.mp_next = &m_back; m_back.mp_prev = &m_head; } + list_impl (const list_impl &&other) + { + swap (other); + } + + list_impl &operator= (const list_impl &&other) + { + if (&other != this) { + swap (other); + } + return *this; + } + list_impl (const list_impl &) { tl_assert (false); } list_impl &operator= (const list_impl &) { tl_assert (false); return *this; } @@ -143,6 +257,18 @@ public: } } + void erase (iterator i) + { + erase (i.operator-> ()); + } + + void erase (iterator from, iterator to) + { + while (from != to) { + erase (from++); + } + } + void swap (list_impl &other) { std::swap (m_head.mp_next, other.m_head.mp_next); @@ -196,14 +322,24 @@ public: delete first (); } - void insert (C *after, C *new_obj) + C *insert (C *after, C *new_obj) { - insert_impl (after, new_obj, true); + return insert_impl (after, new_obj, true); } - void insert_before (C *before, C *new_obj) + iterator insert (iterator after, C *new_obj) { - insert_before_impl (before, new_obj, true); + return iterator (insert (after.operator-> (), new_obj)); + } + + C *insert_before (C *before, C *new_obj) + { + return insert_before_impl (before, new_obj, true); + } + + iterator insert_before (iterator before, C *new_obj) + { + return iterator (insert_before_impl (before.operator-> (), new_obj, true)); } void push_back (C *new_obj) @@ -216,14 +352,24 @@ public: push_front_impl (new_obj, true); } - void insert (C *after, C &new_obj) + C *insert (C *after, C &new_obj) { - insert_impl (after, &new_obj, false); + return insert_impl (after, &new_obj, false); } - void insert_before (C *before, C &new_obj) + iterator insert (iterator after, C &new_obj) { - insert_before_impl (before, &new_obj, false); + return iterator (insert_impl (after.operator-> (), &new_obj, false)); + } + + C *insert_before (C *before, C &new_obj) + { + return insert_before_impl (before, &new_obj, false); + } + + iterator insert_before (iterator before, C &new_obj) + { + return iterator (insert_before_impl (before.operator-> (), &new_obj, false)); } void push_back (C &new_obj) @@ -269,7 +415,7 @@ protected: private: list_node m_head, m_back; - void insert_impl (C *after, C *new_obj, bool owned) + C *insert_impl (C *after, C *new_obj, bool owned) { list_node *after_node = after; if (! after) { @@ -281,9 +427,11 @@ private: after_node->mp_next = new_obj; new_obj->mp_prev = after_node; new_obj->mp_next->mp_prev = new_obj; + + return new_obj; } - void insert_before_impl (C *before, C *new_obj, bool owned) + C *insert_before_impl (C *before, C *new_obj, bool owned) { list_node *before_node = before; if (! before) { @@ -295,6 +443,8 @@ private: before_node->mp_prev = new_obj; new_obj->mp_next = before_node; new_obj->mp_prev->mp_next = new_obj; + + return new_obj; } void push_back_impl (C *new_obj, bool owned) @@ -313,6 +463,11 @@ class list_impl : public list_impl { public: + typedef typename list_impl::iterator iterator; + typedef typename list_impl::const_iterator const_iterator; + typedef typename list_impl::reverse_iterator reverse_iterator; + typedef typename list_impl::const_reverse_iterator const_reverse_iterator; + using list_impl::insert; using list_impl::push_back; using list_impl::pop_back; @@ -339,14 +494,52 @@ public: return *this; } - void insert (C *after, const C &obj) + C *insert (C *after, const C &obj) { - insert (after, new C (obj)); + return insert (after, new C (obj)); } - void insert_before (C *before, const C &obj) + iterator insert (iterator after, const C &obj) { - insert_before (before, new C (obj)); + return insert (after, new C (obj)); + } + + template + iterator insert (iterator after, Iter from, Iter to) + { + if (from == to) { + return after; + } else { + iterator first = this->insert (after, *from++); + for (iterator next = first; from != to; ++from) { + next = this->insert (next, *from); + } + return first; + } + } + + C *insert_before (C *before, const C &obj) + { + return insert_before (before, new C (obj)); + } + + iterator insert_before (iterator before, const C &obj) + { + return insert_before (before, new C (obj)); + } + + template + iterator insert_before (iterator before, Iter from, Iter to) + { + if (from == to) { + return before; + } else { + iterator first = this->insert_before (before, *from++); + for (iterator next = first; from != to; ++from) { + next = this->insert (next, *from); + } + return first; + } } void push_back (const C &obj) @@ -360,72 +553,6 @@ public: } }; -/** - * @brief An iterator for the linked list - */ -template -class list_iterator -{ -public: - typedef std::bidirectional_iterator_tag category; - typedef C value_type; - typedef C &reference; - typedef C *pointer; - - list_iterator (C *p = 0) : mp_p (p) { } - list_iterator operator++ () { mp_p = static_cast (mp_p->mp_next); return *this; } - list_iterator operator-- () { mp_p = static_cast (mp_p->mp_prev); return *this; } - - C *operator-> () const - { - return mp_p; - } - - C &operator* () const - { - return *mp_p; - } - - bool operator== (list_iterator other) const { return mp_p == other.mp_p; } - bool operator!= (list_iterator other) const { return mp_p != other.mp_p; } - -private: - C *mp_p; -}; - -/** - * @brief A reverse iterator for the linked list - */ -template -class reverse_list_iterator -{ -public: - typedef std::bidirectional_iterator_tag category; - typedef C value_type; - typedef C &reference; - typedef C *pointer; - - reverse_list_iterator (C *p = 0) : mp_p (p) { } - reverse_list_iterator operator++ () { mp_p = static_cast (mp_p->mp_prev); return *this; } - reverse_list_iterator operator-- () { mp_p = static_cast (mp_p->mp_next); return *this; } - - C *operator-> () const - { - return mp_p; - } - - C &operator* () const - { - return *mp_p; - } - - bool operator== (reverse_list_iterator other) const { return mp_p == other.mp_p; } - bool operator!= (reverse_list_iterator other) const { return mp_p != other.mp_p; } - -private: - C *mp_p; -}; - /** * @brief A linked list * @@ -448,12 +575,10 @@ class list : public list_impl { public: - typedef list_iterator iterator; - typedef list_iterator const_iterator; - typedef reverse_list_iterator reverse_iterator; - typedef reverse_list_iterator const_reverse_iterator; - - typedef C value_type; + typedef typename list_impl::iterator iterator; + typedef typename list_impl::const_iterator const_iterator; + typedef typename list_impl::reverse_iterator reverse_iterator; + typedef typename list_impl::const_reverse_iterator const_reverse_iterator; using list_impl::first; using list_impl::last; diff --git a/src/tl/tl/tlTimer.cc b/src/tl/tl/tlTimer.cc index d2fa1ba4c..3f74bc4bd 100644 --- a/src/tl/tl/tlTimer.cc +++ b/src/tl/tl/tlTimer.cc @@ -98,14 +98,17 @@ static int64_t ns_time () #elif defined(_MSC_VER) - FILETIME ft; - GetSystemTimeAsFileTime (&ft); + static LARGE_INTEGER freq = { 0 }; - uint64_t t = (uint64_t (ft.dwHighDateTime) << (sizeof (ft.dwHighDateTime) * 8)) | uint64_t (ft.dwLowDateTime); - t -= ft_to_epoch_offset; + if (freq.QuadPart == 0) { + QueryPerformanceFrequency (&freq); + tl_assert (freq.QuadPart > 0); + } - // FILETIME uses 100ns resolution, hence multiply with 100 to get ns: - return int64_t (t) * 100; + LARGE_INTEGER qpc; + QueryPerformanceCounter (&qpc); + + return int64_t (double (qpc.QuadPart) / double (freq.QuadPart) * 1e9 + 0.5); #else diff --git a/src/tl/unit_tests/tlListTests.cc b/src/tl/unit_tests/tlListTests.cc index 06567205d..b0feeb12e 100644 --- a/src/tl/unit_tests/tlListTests.cc +++ b/src/tl/unit_tests/tlListTests.cc @@ -403,3 +403,70 @@ TEST(2_BasicNoCopy) EXPECT_EQ (obj_count, size_t (0)); // mc2 gone as well } + +TEST(3_Insert) +{ + obj_count = 0; + + tl::list l1; + tl::list::iterator i1; + + EXPECT_EQ (l1.empty (), true); + EXPECT_EQ (l1.size (), size_t (0)); + EXPECT_EQ (l2s (l1), ""); + + l1.push_back (MyClass1 (42)); + EXPECT_EQ (l2s (l1), "42"); + EXPECT_EQ (l1.size (), size_t (1)); + + i1 = l1.insert_before (l1.end (), MyClass1 (17)); + EXPECT_EQ (l2s (l1), "42,17"); + EXPECT_EQ (i1->n, 17); + EXPECT_EQ (l1.size (), size_t (2)); + + i1 = l1.insert_before (i1, MyClass1 (11)); + EXPECT_EQ (l2s (l1), "42,11,17"); + EXPECT_EQ (i1->n, 11); + EXPECT_EQ (l1.size (), size_t (3)); + + i1 = l1.insert (i1, MyClass1 (12)); + EXPECT_EQ (l2s (l1), "42,11,12,17"); + EXPECT_EQ (i1->n, 12); + EXPECT_EQ (l1.size (), size_t (4)); + + MyClass1 arr[3] = { MyClass1 (1), MyClass1 (2), MyClass1 (3) }; + + i1 = l1.insert (i1, arr + 0, arr + 0); + EXPECT_EQ (l2s (l1), "42,11,12,17"); + EXPECT_EQ (i1->n, 12); + EXPECT_EQ (l1.size (), size_t (4)); + + i1 = l1.insert (i1, arr + 0, arr + 3); + EXPECT_EQ (l2s (l1), "42,11,12,1,2,3,17"); + EXPECT_EQ (i1->n, 1); + EXPECT_EQ (l1.size (), size_t (7)); + + l1.clear (); + l1.push_back (MyClass1 (42)); + i1 = l1.insert_before (l1.end (), MyClass1 (17)); + EXPECT_EQ (l2s (l1), "42,17"); + EXPECT_EQ (i1->n, 17); + EXPECT_EQ (l1.size (), size_t (2)); + + i1 = l1.insert_before (i1, arr + 0, arr + 0); + EXPECT_EQ (l2s (l1), "42,17"); + EXPECT_EQ (i1->n, 17); + EXPECT_EQ (l1.size (), size_t (2)); + + i1 = l1.insert_before (i1, arr + 0, arr + 3); + EXPECT_EQ (l2s (l1), "42,1,2,3,17"); + EXPECT_EQ (i1->n, 1); + EXPECT_EQ (l1.size (), size_t (5)); + + // test erase range + l1.erase (i1, l1.end ()); + EXPECT_EQ (l2s (l1), "42"); + EXPECT_EQ (l1.size (), size_t (1)); +} + + From f1f3c64ab106952647fe5a8021b00fb06c66a9a1 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 7 Sep 2024 21:27:50 +0200 Subject: [PATCH 4/6] Mute noisy diagnostics with Python debug builds, avoid post-Finalize Python API access by pre-Finalize cleanup of modules. This avoids an assertion in Python debug builds and is a better style anyway. --- src/pya/pya/pya.cc | 21 +++++++++++++++++++++ src/pya/pya/pya.h | 10 +++++++++- src/pya/pya/pyaCallables.cc | 3 +++ src/pya/pya/pyaModule.cc | 16 ++++++++++------ src/pya/pya/pyaModule.h | 6 ++++++ src/pymod/pymodHelper.h | 14 ++++++++++---- 6 files changed, 59 insertions(+), 11 deletions(-) diff --git a/src/pya/pya/pya.cc b/src/pya/pya/pya.cc index 5f177fa56..0de6ea55c 100644 --- a/src/pya/pya/pya.cc +++ b/src/pya/pya/pya.cc @@ -360,6 +360,10 @@ PythonInterpreter::PythonInterpreter (bool embedded) PythonInterpreter::~PythonInterpreter () { + for (auto m = m_modules.begin (); m != m_modules.end (); ++m) { + (*m)->cleanup (); + } + m_stdout_channel = PythonRef (); m_stderr_channel = PythonRef (); m_stdout = PythonPtr (); @@ -370,6 +374,23 @@ PythonInterpreter::~PythonInterpreter () if (m_embedded) { Py_Finalize (); } + + for (auto m = m_modules.begin (); m != m_modules.end (); ++m) { + delete *m; + } + m_modules.clear (); +} + +void +PythonInterpreter::register_module (pya::PythonModule *module) +{ + for (auto m = m_modules.begin (); m != m_modules.end (); ++m) { + if (*m == module) { + return; // already registered + } + } + + m_modules.push_back (module); } char * diff --git a/src/pya/pya/pya.h b/src/pya/pya/pya.h index ee8d35381..76838f4f0 100644 --- a/src/pya/pya/pya.h +++ b/src/pya/pya/pya.h @@ -105,6 +105,14 @@ public: */ ~PythonInterpreter (); + /** + * @brief Registers a module + * + * The registered modules are cleaned up before the interpreter shuts down. The interpreter takes + * ownership of the module object. + */ + void register_module (pya::PythonModule *module); + /** * @brief Add the given path to the search path */ @@ -279,7 +287,7 @@ private: std::map m_file_id_map; std::wstring mp_py3_app_name; bool m_embedded; - std::unique_ptr m_pya_module; + std::vector m_modules; }; } diff --git a/src/pya/pya/pyaCallables.cc b/src/pya/pya/pyaCallables.cc index 5dfc3e4f1..d12ea6905 100644 --- a/src/pya/pya/pyaCallables.cc +++ b/src/pya/pya/pyaCallables.cc @@ -51,6 +51,9 @@ pya_object_deallocate (PyObject *self) // we better work around it. ++self->ob_refcnt; + // Mute Python warnings in debug case + PyObject_GC_UnTrack (self); + PYAObjectBase *p = PYAObjectBase::from_pyobject (self); p->~PYAObjectBase (); Py_TYPE (self)->tp_free (self); diff --git a/src/pya/pya/pyaModule.cc b/src/pya/pya/pyaModule.cc index 30452d669..62e6bacde 100644 --- a/src/pya/pya/pyaModule.cc +++ b/src/pya/pya/pyaModule.cc @@ -71,12 +71,6 @@ PythonModule::PythonModule () PythonModule::~PythonModule () { - PYAObjectBase::clear_callbacks_cache (); - - // the Python objects were probably deleted by Python itself as it exited - - // don't try to delete them again. - mp_module.release (); - while (!m_methods_heap.empty ()) { delete m_methods_heap.back (); m_methods_heap.pop_back (); @@ -93,6 +87,16 @@ PythonModule::~PythonModule () } } +void +PythonModule::cleanup () +{ + // the Python objects are probably deleted by Python itself as it exits - + // don't try to delete them again in the destructor. + mp_module.release (); + + PYAObjectBase::clear_callbacks_cache (); +} + PyObject * PythonModule::module () { diff --git a/src/pya/pya/pyaModule.h b/src/pya/pya/pyaModule.h index 0adcc9586..b543ae02b 100644 --- a/src/pya/pya/pyaModule.h +++ b/src/pya/pya/pyaModule.h @@ -65,6 +65,12 @@ public: */ ~PythonModule (); + /** + * @brief Clean up the module + * This method is called by the interpreter before Py_Finalize + */ + void cleanup (); + /** * @brief Initializes the module * This entry point is for external use where the module has not been created yet diff --git a/src/pymod/pymodHelper.h b/src/pymod/pymodHelper.h index 55bc56185..6d116e234 100644 --- a/src/pymod/pymodHelper.h +++ b/src/pymod/pymodHelper.h @@ -34,6 +34,7 @@ #include "pyaModule.h" #include "pyaUtils.h" +#include "pya.h" #include "gsi.h" #include "gsiExpression.h" @@ -41,7 +42,12 @@ static PyObject * module_init (const char *pymod_name, const char *mod_name, const char *mod_description) { - static pya::PythonModule module; + if (! pya::PythonInterpreter::instance ()) { + return 0; + } + + pya::PythonModule *module = new pya::PythonModule (); + pya::PythonInterpreter::instance ()->register_module (module); PYA_TRY @@ -50,10 +56,10 @@ module_init (const char *pymod_name, const char *mod_name, const char *mod_descr // required for the tiling processor for example gsi::initialize_expressions (); - module.init (pymod_name, mod_description); - module.make_classes (mod_name); + module->init (pymod_name, mod_description); + module->make_classes (mod_name); - return module.take_module (); + return module->take_module (); PYA_CATCH_ANYWHERE From 16b6328f1cc36cec42bbcaf949f83b44bfb35a37 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 8 Sep 2024 00:24:56 +0200 Subject: [PATCH 5/6] Fixed new Python module handling for standalone module case --- src/pya/pya/pya.cc | 4 +++- src/pya/pya/pyaModule.cc | 2 -- src/pya/pya/pyaObject.cc | 10 +++++++++- src/pya/pya/pyaObject.h | 2 +- src/pya/pya/pyaRefs.cc | 33 +++++++++++++++++++++++++-------- src/pya/pya/pyaRefs.h | 9 +++++++++ src/pymod/pymodHelper.h | 16 ++++++++-------- 7 files changed, 55 insertions(+), 21 deletions(-) diff --git a/src/pya/pya/pya.cc b/src/pya/pya/pya.cc index 0de6ea55c..bc544e7f7 100644 --- a/src/pya/pya/pya.cc +++ b/src/pya/pya/pya.cc @@ -192,7 +192,7 @@ PythonInterpreter::PythonInterpreter (bool embedded) sp_interpreter = this; - // this monitor whether Python shuts down and deletes the interpreter's + // this monitors whether Python shuts down and deletes the interpreter's // instance. // NOTE: this assumes, the interpreter was created with new(!) Py_AtExit (&reset_interpreter); @@ -364,6 +364,8 @@ PythonInterpreter::~PythonInterpreter () (*m)->cleanup (); } + PYAObjectBase::clear_callbacks_cache (m_embedded); + m_stdout_channel = PythonRef (); m_stderr_channel = PythonRef (); m_stdout = PythonPtr (); diff --git a/src/pya/pya/pyaModule.cc b/src/pya/pya/pyaModule.cc index 62e6bacde..86fa88fba 100644 --- a/src/pya/pya/pyaModule.cc +++ b/src/pya/pya/pyaModule.cc @@ -93,8 +93,6 @@ PythonModule::cleanup () // the Python objects are probably deleted by Python itself as it exits - // don't try to delete them again in the destructor. mp_module.release (); - - PYAObjectBase::clear_callbacks_cache (); } PyObject * diff --git a/src/pya/pya/pyaObject.cc b/src/pya/pya/pyaObject.cc index 3887ed3a3..2bdc27183 100644 --- a/src/pya/pya/pyaObject.cc +++ b/src/pya/pya/pyaObject.cc @@ -514,8 +514,16 @@ PYAObjectBase::initialize_callbacks () } void -PYAObjectBase::clear_callbacks_cache () +PYAObjectBase::clear_callbacks_cache (bool embedded) { + // if not embedded, we cannot use the python API at this stage - do not try to + // reference count the objects there. + if (! embedded) { + for (auto c = s_callbacks_cache.begin (); c != s_callbacks_cache.end (); ++c) { + c->first.release_const (); + } + } + s_callbacks_cache.clear (); } diff --git a/src/pya/pya/pyaObject.h b/src/pya/pya/pyaObject.h index 96febf155..86344a328 100644 --- a/src/pya/pya/pyaObject.h +++ b/src/pya/pya/pyaObject.h @@ -200,7 +200,7 @@ public: /** * @brief Clears the callbacks cache */ - static void clear_callbacks_cache (); + static void clear_callbacks_cache (bool embedded); private: friend class StatusChangedListener; diff --git a/src/pya/pya/pyaRefs.cc b/src/pya/pya/pyaRefs.cc index 49dc7049f..420aa88ee 100644 --- a/src/pya/pya/pyaRefs.cc +++ b/src/pya/pya/pyaRefs.cc @@ -32,19 +32,19 @@ namespace pya // PythonRef implementation PythonRef::PythonRef () - : mp_obj (NULL) + : mp_obj (NULL), m_owns_pointer (true) { // .. nothing yet .. } PythonRef::PythonRef (const PythonPtr &ptr) - : mp_obj (ptr.get ()) + : mp_obj (ptr.get ()), m_owns_pointer (true) { Py_XINCREF (mp_obj); } PythonRef::PythonRef (PyObject *obj, bool new_ref) - : mp_obj (obj) + : mp_obj (obj), m_owns_pointer (true) { if (! new_ref) { Py_XINCREF (mp_obj); @@ -53,38 +53,49 @@ PythonRef::PythonRef (PyObject *obj, bool new_ref) PythonRef &PythonRef::operator= (PyObject *obj) { - Py_XDECREF (mp_obj); + if (m_owns_pointer) { + Py_XDECREF (mp_obj); + } mp_obj = obj; + m_owns_pointer = true; return *this; } PythonRef &PythonRef::operator= (const PythonPtr &ptr) { - Py_XDECREF (mp_obj); + if (m_owns_pointer) { + Py_XDECREF (mp_obj); + } mp_obj = ptr.get (); Py_XINCREF (mp_obj); + m_owns_pointer = true; return *this; } PythonRef &PythonRef::operator= (const PythonRef &other) { if (this != &other && mp_obj != other.mp_obj) { - Py_XDECREF (mp_obj); + if (m_owns_pointer) { + Py_XDECREF (mp_obj); + } mp_obj = other.mp_obj; + m_owns_pointer = true; Py_XINCREF (mp_obj); } return *this; } PythonRef::PythonRef (const PythonRef &other) - : mp_obj (other.mp_obj) + : mp_obj (other.mp_obj), m_owns_pointer (true) { Py_XINCREF (mp_obj); } PythonRef::~PythonRef () { - Py_XDECREF (mp_obj); + if (m_owns_pointer) { + Py_XDECREF (mp_obj); + } } PythonRef::operator bool () const @@ -109,6 +120,12 @@ PyObject *PythonRef::release () return o; } +PyObject *PythonRef::release_const () const +{ + m_owns_pointer = false; + return mp_obj; +} + // -------------------------------------------------------------------------- // PythonPtr implementation diff --git a/src/pya/pya/pyaRefs.h b/src/pya/pya/pyaRefs.h index 460aa7b37..c04673ae9 100644 --- a/src/pya/pya/pyaRefs.h +++ b/src/pya/pya/pyaRefs.h @@ -116,6 +116,14 @@ public: */ PyObject *release (); + /** + * @brief Takes the pointer, but does not change the value + * This method will stop the reference from managing the object, but + * maintains the pointer. Do not access the pointer after this + * operation. + */ + PyObject *release_const () const; + /** * @brief Comparison operator */ @@ -134,6 +142,7 @@ public: private: PyObject *mp_obj; + mutable bool m_owns_pointer; }; /** diff --git a/src/pymod/pymodHelper.h b/src/pymod/pymodHelper.h index 6d116e234..22557ed93 100644 --- a/src/pymod/pymodHelper.h +++ b/src/pymod/pymodHelper.h @@ -42,12 +42,7 @@ static PyObject * module_init (const char *pymod_name, const char *mod_name, const char *mod_description) { - if (! pya::PythonInterpreter::instance ()) { - return 0; - } - - pya::PythonModule *module = new pya::PythonModule (); - pya::PythonInterpreter::instance ()->register_module (module); + std::unique_ptr module (new pya::PythonModule ()); PYA_TRY @@ -59,10 +54,15 @@ module_init (const char *pymod_name, const char *mod_name, const char *mod_descr module->init (pymod_name, mod_description); module->make_classes (mod_name); - return module->take_module (); + PyObject *mod_object = module->take_module (); + + tl_assert (pya::PythonInterpreter::instance () != 0); + pya::PythonInterpreter::instance ()->register_module (module.release ()); + + return mod_object; PYA_CATCH_ANYWHERE - + return 0; } From 97dcf0aa0b4b22341b0e4b50278e15044fb82b9b Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 14 Sep 2024 16:21:07 +0200 Subject: [PATCH 6/6] Porting polygon generator from std::deque to std::list with support by 'splice' ... should fix the problem --- src/db/db/dbPolygonGenerators.cc | 90 +++++++++++++++++++++++--------- 1 file changed, 65 insertions(+), 25 deletions(-) diff --git a/src/db/db/dbPolygonGenerators.cc b/src/db/db/dbPolygonGenerators.cc index 158c98905..ddaf33268 100644 --- a/src/db/db/dbPolygonGenerators.cc +++ b/src/db/db/dbPolygonGenerators.cc @@ -52,7 +52,7 @@ struct PGPoint class PGPolyContour { public: - typedef std::deque contour_type; + typedef std::list contour_type; typedef contour_type::const_iterator const_iterator; typedef contour_type::iterator iterator; @@ -172,6 +172,13 @@ public: m_contour.erase (from, to); } + void splice (iterator at, PGPolyContour &contour) + { + m_size += contour.size (); + contour.m_size = 0; + m_contour.splice (at, contour.m_contour); + } + template iterator insert (iterator at, I from, I to) { @@ -194,8 +201,43 @@ private: size_t m_size; }; +static inline +PGPolyContour::const_iterator operator+ (PGPolyContour::const_iterator i, int n) +{ + while (n-- > 0) { + ++i; + } + return i; +} -class PGContourList +static inline +PGPolyContour::iterator operator+ (PGPolyContour::iterator i, int n) +{ + while (n-- > 0) { + ++i; + } + return i; +} + +static inline +PGPolyContour::const_iterator operator- (PGPolyContour::const_iterator i, int n) +{ + while (n-- > 0) { + --i; + } + return i; +} + +static inline +PGPolyContour::iterator operator- (PGPolyContour::iterator i, int n) +{ + while (n-- > 0) { + --i; + } + return i; +} + +class PGContourList { public: PGContourList () @@ -528,7 +570,7 @@ void PolygonGenerator::eliminate_hole () tl_assert (cprev.size () >= 2); // Compute intersection point with next edge - db::Edge eprev (cprev.end ()[-2], cprev.back ()); + db::Edge eprev (*(cprev.end () - 2), cprev.back ()); db::Coord xprev = db::coord_traits::rounded (edge_xaty (eprev, m_y)); db::Point pprev (xprev, m_y); @@ -538,7 +580,7 @@ void PolygonGenerator::eliminate_hole () cc.is_hole (false); cc.push_back (c.front ()); - cc.push_back (c.begin ()[1]); + cc.push_back (*(c.begin () + 1)); if (pprev != cc.back ()) { cc.push_back (pprev); } @@ -547,7 +589,7 @@ void PolygonGenerator::eliminate_hole () } cprev.back () = pprev; - while (cprev.size () > 2 && cprev.back ().y () == m_y && cprev.end ()[-2].y () == m_y && cprev.back ().x () <= cprev.end ()[-2].x ()) { + while (cprev.size () > 2 && cprev.back ().y () == m_y && (cprev.end () - 2)->y () == m_y && cprev.back ().x () <= (cprev.end () - 2)->x ()) { cprev.pop_back (); } cprev.insert (cprev.end (), c.end () - 2, c.end ()); @@ -638,7 +680,7 @@ PolygonGenerator::join_contours (db::Coord x) tl_assert (cprev.size () >= 2); // compute intersection point with next edge - db::Edge eprev (cprev.end ()[-2], cprev.back ()); + db::Edge eprev (*(cprev.end () - 2), cprev.back ()); db::Coord xprev = db::coord_traits::rounded (edge_xaty (eprev, m_y)); db::Point pprev (xprev, m_y); @@ -646,13 +688,13 @@ PolygonGenerator::join_contours (db::Coord x) tl_assert (c2.size () >= 2); cprev.back () = pprev; - while (cprev.size () > 2 && cprev.back ().y () == m_y && cprev.end ()[-2].y () == m_y && cprev.back ().x () <= cprev.end ()[-2].x ()) { + while (cprev.size () > 2 && cprev.back ().y () == m_y && (cprev.end () - 2)->y () == m_y && cprev.back ().x () <= (cprev.end () - 2)->x ()) { cprev.pop_back (); } if (iprev == i1) { - if (cprev.begin ()->y () == m_y && cprev.begin ()[1].y () == m_y && cprev.front ().x () >= cprev.begin ()[1].x ()) { + if (cprev.front ().y () == m_y && (cprev.begin () + 1)->y () == m_y && cprev.front ().x () >= (cprev.begin () + 1)->x ()) { cprev.front () = cprev.back (); } else { cprev.push_front (cprev.back ()); @@ -662,7 +704,7 @@ PolygonGenerator::join_contours (db::Coord x) } else { - cprev.insert (cprev.end (), c1.begin (), c1.end ()); + cprev.splice (cprev.end (), c1); cprev.is_hole (false); mp_contours->join (iprev, i1); @@ -705,20 +747,16 @@ PolygonGenerator::join_contours (db::Coord x) } else if (! m_open_pos->first && ! n->first) { -{ - tl::SelfTimer timer ("PGContour::insert"); // remove c1 from list of contours, join with c2 if (c2.is_hole ()) { - c2.insert (c2.end (), c1.begin () + 1, c1.end ()); + c1.pop_front (); + c2.splice (c2.end (), c1); } else { - c2.insert (c2.begin (), c1.begin (), c1.end () - 1); + c1.pop_back (); + c2.splice (c2.begin (), c1); } -} -{ - tl::SelfTimer timer ("join_contours"); mp_contours->join (i2, i1); -} open_map_iterator_type o = m_open_pos; do { @@ -732,9 +770,11 @@ PolygonGenerator::join_contours (db::Coord x) // remove c1 from list of contours, join with c2 if (c2.is_hole ()) { // yes! c2 is correct! - c1.insert (c1.end (), c2.begin () + 1, c2.end ()); + c2.pop_front (); + c1.splice (c1.end (), c2); } else { - c1.insert (c1.begin (), c2.begin (), c2.end () - 1); + c2.pop_back (); + c1.splice (c1.begin (), c2); } mp_contours->join (i1, i2); @@ -787,8 +827,8 @@ PolygonGenerator::join_contours (db::Coord x) // shallow analysis: insert the cutline at the end of the sequence - this may // cut lines collinear with contour edges - eprev = db::Edge (ins[-2], ins[-1]); - xprev = db::coord_traits::rounded (edge_xaty (db::Edge (ins[-2], ins[-1]), m_y)); + eprev = db::Edge (*(ins - 2), *(ins - 1)); + xprev = db::coord_traits::rounded (edge_xaty (db::Edge (*(ins - 2), *(ins - 1)), m_y)); #else // deep analysis: determine insertion point: pick the one where the cutline is shortest @@ -810,17 +850,17 @@ PolygonGenerator::join_contours (db::Coord x) db::Point pprev (xprev, m_y); // remove collinear edges along the cut line - ins[-1] = pprev; - while (ins - cprev.begin () > 1 && ins[-2].y () == m_y && ins[-1].y () == m_y) { + *(ins - 1) = pprev; + while (ins - 1 != cprev.begin () && (ins - 2)->y () == m_y && (ins - 1)->y () == m_y) { ins = cprev.erase (ins - 1); } if ((c1.begin () + 1)->y () == m_y) { ins = cprev.insert (ins, c1.begin () + 1, c1.end ()); - ins += c1.size () - 1; + ins = ins + (c1.size () - 1); } else { ins = cprev.insert (ins, c1.begin (), c1.end ()); - ins += c1.size (); + ins = ins + c1.size (); } ins = cprev.insert (ins, pprev);