From d5725c3ed51a7d9bc7e56103a4fa8d1efcebd30f Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Tue, 22 Nov 2022 22:06:10 +0100 Subject: [PATCH 01/10] DRC performance issue fix attempt Problem was that destruction of database objects was postponed during callbacks, specifically the interpreter callback. This caused db::Region objects to pile up. The original feature was introduced to prevent issues when Ruby/C++ code interacts inside Qt UIs. The new fix does not disable GC, but protects Ruby objects during method calls. Maybe that solves the problem too. --- src/rba/rba/rba.cc | 4 ++ src/rba/rba/rbaInternal.cc | 90 +------------------------------------- src/rba/rba/rbaInternal.h | 25 +++++++++++ 3 files changed, 30 insertions(+), 89 deletions(-) diff --git a/src/rba/rba/rba.cc b/src/rba/rba/rba.cc index c4099cc0a..2432f01d7 100644 --- a/src/rba/rba/rba.cc +++ b/src/rba/rba/rba.cc @@ -1001,6 +1001,10 @@ method_adaptor (int mid, int argc, VALUE *argv, VALUE self, bool ctor) const gsi::ClassBase *cls_decl; Proxy *p = 0; + // this prevents side effects of callbacks raised from within the called functions - + // if these trigger the GC, self is protected from destruction herein. + GCLocker gc_locker (self); + if (TYPE (self) == T_CLASS) { // we have a static method cls_decl = find_cclass (self); diff --git a/src/rba/rba/rbaInternal.cc b/src/rba/rba/rbaInternal.cc index bcec542b7..55bbefe6d 100644 --- a/src/rba/rba/rbaInternal.cc +++ b/src/rba/rba/rbaInternal.cc @@ -158,90 +158,6 @@ gc_unlock_object (VALUE value) } } -// -------------------------------------------------------------------------- - -/** - * @brief A final finalizer - * Final destruction of finalized objects is delegated to this class. - * It's main purpose is to temporarily disable C++ destruction while - * in a callback. Deleting C++ objects in a callback leads to unpredictable - * side effects and must be avoided. - */ -class InternalGC -{ -public: - InternalGC () - : m_enabled (true) - { - // .. nothing yet .. - } - - /** - * @brief Enables or disables the GC - * Returns the previous state - */ - bool enable (bool e) - { - std::swap (e, m_enabled); - return e; - } - - /** - * @brief Queues (if disabled) or deletes the given object (plus all queued ones) if enabled - */ - void destroy_maybe (const gsi::ClassBase *cls, void *obj) - { - if (! m_enabled) { - m_queue.push_back (std::make_pair (cls, obj)); - } else { - m_queue.clear (); - cls->destroy (obj); - for (std::vector >::const_iterator q = m_queue.begin (); q != m_queue.end (); ++q) { - q->first->destroy (q->second); - } - } - } - -private: - std::vector > m_queue; - bool m_enabled; -}; - -static InternalGC s_gc; - -/** - * @brief Destroys the object through the internal GC - */ -inline void destroy_object (const gsi::ClassBase *cls, void *obj) -{ - s_gc.destroy_maybe (cls, obj); -} - -/** - * @brief A GC disabler - * By instantiating this object, the gc is disabled while the object is alive. - * Specifically in callbacks it's important to disable the GC to avoid undesired - * side effects. - */ -class GCDisabler -{ -public: - GCDisabler (InternalGC *gc = 0) - : mp_gc (gc ? gc : &s_gc) - { - m_was_enabled = mp_gc->enable (false); - } - - ~GCDisabler() - { - mp_gc->enable (m_was_enabled); - } - -private: - bool m_was_enabled; - InternalGC *mp_gc; -}; - // -------------------------------------------------------------------------- // Proxy implementation @@ -309,8 +225,6 @@ bool Proxy::can_call () const void Proxy::call (int id, gsi::SerialArgs &args, gsi::SerialArgs &ret) const { - GCDisabler gc_disabler; - tl_assert (id < int (m_cbfuncs.size ()) && id >= 0); const gsi::MethodBase *meth = m_cbfuncs [id].method; @@ -635,7 +549,7 @@ Proxy::set (void *obj, bool owned, bool const_ref, bool can_destroy, VALUE self) // Destroy the object if we are owner. We don't destroy the object if it was locked // (either because we are not owner or from C++ side using keep()) if (prev_owned) { - destroy_object (cls, prev_obj); + cls->destroy (prev_obj); } } @@ -891,8 +805,6 @@ SignalHandler::define_class (VALUE module, const char *name) void SignalHandler::call (const gsi::MethodBase *meth, gsi::SerialArgs &args, gsi::SerialArgs &ret) const { - GCDisabler gc_disabler; - VALUE argv = rb_ary_new2 (long (std::distance (meth->begin_arguments (), meth->end_arguments ()))); RB_GC_GUARD (argv); diff --git a/src/rba/rba/rbaInternal.h b/src/rba/rba/rbaInternal.h index ccad2275e..21949105e 100644 --- a/src/rba/rba/rbaInternal.h +++ b/src/rba/rba/rbaInternal.h @@ -251,6 +251,31 @@ void gc_lock_object (VALUE value); */ void gc_unlock_object (VALUE value); +/** + * @brief A Locker for the object based on the RIIA pattern + */ +class GCLocker +{ +public: + GCLocker (VALUE value) + : m_value (value) + { + gc_lock_object (m_value); + } + + ~GCLocker () + { + gc_unlock_object (m_value); + } + +private: + GCLocker (); + GCLocker (const GCLocker &other); + GCLocker &operator= (const GCLocker &other); + + VALUE m_value; +}; + /** * @brief Makes the locked object vault required for gc_lock_object and gc_unlock_object * From 86cc523c773628021f5c7bdc48dad2f897a89ccf Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Wed, 23 Nov 2022 00:33:37 +0100 Subject: [PATCH 02/10] Bugfixed GCLocker implementation - is now recursive --- src/rba/rba/rbaInternal.cc | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/rba/rba/rbaInternal.cc b/src/rba/rba/rbaInternal.cc index 55bbefe6d..92b79e521 100644 --- a/src/rba/rba/rbaInternal.cc +++ b/src/rba/rba/rbaInternal.cc @@ -54,7 +54,7 @@ public: void mark_this (); private: - std::set m_objects; + std::map m_objects; static VALUE m_klass; static VALUE m_instance; @@ -84,20 +84,31 @@ LockedObjectVault::~LockedObjectVault () void LockedObjectVault::add (VALUE object) { - m_objects.insert (object); + auto i = m_objects.find (object); + if (i != m_objects.end ()) { + i->second += 1; + } else { + m_objects.insert (std::make_pair (object, size_t (1))); + } } void LockedObjectVault::remove (VALUE object) { - m_objects.erase (object); + auto i = m_objects.find (object); + if (i != m_objects.end ()) { + i->second -= 1; + if (i->second == 0) { + m_objects.erase (i); + } + } } void LockedObjectVault::mark_this () { - for (std::set::iterator o = m_objects.begin (); o != m_objects.end (); ++o) { - rb_gc_mark (*o); + for (auto o = m_objects.begin (); o != m_objects.end (); ++o) { + rb_gc_mark (o->first); } } From 7431ec6f43d494e85a5da0939bfb2f2797f278b5 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Wed, 23 Nov 2022 00:36:08 +0100 Subject: [PATCH 03/10] Solved an ownership issue in DRC which popped up after universal DRC nodes got deleted. --- src/drc/drc/built-in-macros/_drc_complex_ops.rb | 12 ++++++------ .../drc/built-in-macros/_drc_cop_integration.rb | 14 ++++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/drc/drc/built-in-macros/_drc_complex_ops.rb b/src/drc/drc/built-in-macros/_drc_complex_ops.rb index e83f71d60..5014cf5ef 100644 --- a/src/drc/drc/built-in-macros/_drc_complex_ops.rb +++ b/src/drc/drc/built-in-macros/_drc_complex_ops.rb @@ -232,15 +232,15 @@ class DRCOpNode attr_accessor :description attr_accessor :engine - def initialize(engine, node = nil) - @node = node + def initialize(engine, &factory) + @factory = factory self.engine = engine self.description = "Basic" end def create_node(cache) n = cache[self.object_id] - if !n + if !n || n.destroyed? n = self.do_create_node(cache) cache[self.object_id] = n end @@ -248,7 +248,7 @@ class DRCOpNode end def do_create_node(cache) - @node + @factory.call end def dump(indent) @@ -362,8 +362,8 @@ CODE return self.inverted else # TODO: what if the expression isn't region? - empty = RBA::CompoundRegionOperationNode::new_empty(RBA::CompoundRegionOperationNode::ResultType::Region) - DRCOpNodeCase::new(@engine, [ self, DRCOpNode::new(@engine, empty), @engine.primary ]) + empty = DRCOpNode::new(@engine) { RBA::CompoundRegionOperationNode::new_empty(RBA::CompoundRegionOperationNode::ResultType::Region) } + DRCOpNodeCase::new(@engine, [ self, empty, @engine.primary ]) end end end diff --git a/src/drc/drc/built-in-macros/_drc_cop_integration.rb b/src/drc/drc/built-in-macros/_drc_cop_integration.rb index e299350f7..2f47e052f 100644 --- a/src/drc/drc/built-in-macros/_drc_cop_integration.rb +++ b/src/drc/drc/built-in-macros/_drc_cop_integration.rb @@ -476,7 +476,7 @@ module DRC layer.requires_region - res = DRCOpNode::new(self, RBA::CompoundRegionOperationNode::new_secondary(layer.data)) + res = DRCOpNode::new(self) { RBA::CompoundRegionOperationNode::new_secondary(layer.data) } res.description = "secondary" return res @@ -493,7 +493,7 @@ module DRC # is called on. def primary - res = DRCOpNode::new(self, RBA::CompoundRegionOperationNode::new_primary) + res = DRCOpNode::new(self) { RBA::CompoundRegionOperationNode::new_primary } res.description = "primary" return res end @@ -520,7 +520,7 @@ module DRC # @/code def foreign - res = DRCOpNode::new(self, RBA::CompoundRegionOperationNode::new_foreign) + res = DRCOpNode::new(self) { RBA::CompoundRegionOperationNode::new_foreign } res.description = "foreign" return res end @@ -1555,10 +1555,12 @@ CODE def _make_node(arg) if arg.is_a?(DRCLayer) - arg = DRCOpNode::new(self, RBA::CompoundRegionOperationNode::new_secondary(arg.data)) - arg.description = "secondary" + node = DRCOpNode::new(self) { RBA::CompoundRegionOperationNode::new_secondary(arg.data) } + node.description = "secondary" + return node + else + return arg end - arg end end From e04875b4e0425e70099ffc9f641a4e5fcddd92b6 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Wed, 23 Nov 2022 01:03:36 +0100 Subject: [PATCH 04/10] Fixed a problem with object lifetime and tiling processor input found after GC became active. Added a documentation hint. --- src/db/db/gsiDeclDbTilingProcessor.cc | 24 +++++++++++++++++++++++ src/drc/drc/built-in-macros/_drc_layer.rb | 6 ++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/db/db/gsiDeclDbTilingProcessor.cc b/src/db/db/gsiDeclDbTilingProcessor.cc index 39b17def5..e3a3e26db 100644 --- a/src/db/db/gsiDeclDbTilingProcessor.cc +++ b/src/db/db/gsiDeclDbTilingProcessor.cc @@ -417,6 +417,9 @@ Class decl_TilingProcessor ("db", "TilingProcessor", "Regions don't always come with a database unit, hence a database unit should be specified with the \\dbu= method unless " "a layout object is specified as input too.\n" "\n" + "Caution: the Region object must stay valid during the lifetime of the tiling processor. Take care to store it in " + "a variable to prevent early destruction of the Region object. Not doing so may crash the application.\n" + "\n" "The name specifies the variable under which the input can be used in the scripts." ) + method_ext ("input", &tp_input9, gsi::arg ("name"), gsi::arg ("region"), gsi::arg ("trans"), @@ -425,6 +428,9 @@ Class decl_TilingProcessor ("db", "TilingProcessor", "Regions don't always come with a database unit, hence a database unit should be specified with the \\dbu= method unless " "a layout object is specified as input too.\n" "\n" + "Caution: the Region object must stay valid during the lifetime of the tiling processor. Take care to store it in " + "a variable to prevent early destruction of the Region object. Not doing so may crash the application.\n" + "\n" "The name specifies the variable under which the input can be used in the scripts." "\n" "This variant allows one to specify an additional transformation too. It has been introduced in version 0.23.2.\n" @@ -435,6 +441,9 @@ Class decl_TilingProcessor ("db", "TilingProcessor", "Edge collections don't always come with a database unit, hence a database unit should be specified with the \\dbu= method unless " "a layout object is specified as input too.\n" "\n" + "Caution: the Edges object must stay valid during the lifetime of the tiling processor. Take care to store it in " + "a variable to prevent early destruction of the Edges object. Not doing so may crash the application.\n" + "\n" "The name specifies the variable under which the input can be used in the scripts." ) + method_ext ("input", &tp_input11, gsi::arg ("name"), gsi::arg ("edges"), gsi::arg ("trans"), @@ -443,6 +452,9 @@ Class decl_TilingProcessor ("db", "TilingProcessor", "Edge collections don't always come with a database unit, hence a database unit should be specified with the \\dbu= method unless " "a layout object is specified as input too.\n" "\n" + "Caution: the Edges object must stay valid during the lifetime of the tiling processor. Take care to store it in " + "a variable to prevent early destruction of the Edges object. Not doing so may crash the application.\n" + "\n" "The name specifies the variable under which the input can be used in the scripts." "\n" "This variant allows one to specify an additional transformation too. It has been introduced in version 0.23.2.\n" @@ -454,6 +466,9 @@ Class decl_TilingProcessor ("db", "TilingProcessor", "Edge pair collections don't always come with a database unit, hence a database unit should be specified with the \\dbu= method unless " "a layout object is specified as input too.\n" "\n" + "Caution: the EdgePairs object must stay valid during the lifetime of the tiling processor. Take care to store it in " + "a variable to prevent early destruction of the EdgePairs object. Not doing so may crash the application.\n" + "\n" "The name specifies the variable under which the input can be used in the scripts." "\n" "This variant has been introduced in version 0.27.\n" @@ -464,6 +479,9 @@ Class decl_TilingProcessor ("db", "TilingProcessor", "Edge pair collections don't always come with a database unit, hence a database unit should be specified with the \\dbu= method unless " "a layout object is specified as input too.\n" "\n" + "Caution: the EdgePairs object must stay valid during the lifetime of the tiling processor. Take care to store it in " + "a variable to prevent early destruction of the EdgePairs object. Not doing so may crash the application.\n" + "\n" "The name specifies the variable under which the input can be used in the scripts." "\n" "This variant has been introduced in version 0.27.\n" @@ -474,6 +492,9 @@ Class decl_TilingProcessor ("db", "TilingProcessor", "Text collections don't always come with a database unit, hence a database unit should be specified with the \\dbu= method unless " "a layout object is specified as input too.\n" "\n" + "Caution: the Texts object must stay valid during the lifetime of the tiling processor. Take care to store it in " + "a variable to prevent early destruction of the Texts object. Not doing so may crash the application.\n" + "\n" "The name specifies the variable under which the input can be used in the scripts." "\n" "This variant has been introduced in version 0.27.\n" @@ -484,6 +505,9 @@ Class decl_TilingProcessor ("db", "TilingProcessor", "Text collections don't always come with a database unit, hence a database unit should be specified with the \\dbu= method unless " "a layout object is specified as input too.\n" "\n" + "Caution: the Texts object must stay valid during the lifetime of the tiling processor. Take care to store it in " + "a variable to prevent early destruction of the Texts object. Not doing so may crash the application.\n" + "\n" "The name specifies the variable under which the input can be used in the scripts." "\n" "This variant has been introduced in version 0.27.\n" diff --git a/src/drc/drc/built-in-macros/_drc_layer.rb b/src/drc/drc/built-in-macros/_drc_layer.rb index c8e2731a5..76ed6204e 100644 --- a/src/drc/drc/built-in-macros/_drc_layer.rb +++ b/src/drc/drc/built-in-macros/_drc_layer.rb @@ -4222,11 +4222,13 @@ CODE tp.output("res", res) tp.input("input", self.data) tp.threads = (@engine.threads || 1) + if tile_boundary - tp.input("boundary", tile_boundary.data) + boundary = tile_boundary.data else - tp.input("boundary", RBA::Region::new(self.data.bbox)) + boundary = RBA::Region::new(self.data.bbox) end + tp.input("boundary", boundary) tp.var("vmin", limits[0] || 0.0) tp.var("vmax", limits[1] || 1.0) From 53d6e8d3cb67d719a75c43b0b765b3f300a51825 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Thu, 24 Nov 2022 20:41:19 +0100 Subject: [PATCH 05/10] DSS now keeps layouts (avoids excessive cell mapping), frequent GC in DRC tests for heavy GC load testing --- src/buddies/src/bd/strmxor.cc | 1 - src/db/db/dbDeepShapeStore.cc | 4 +-- src/drc/drc/built-in-macros/_drc_engine.rb | 12 ++++++++- src/drc/unit_tests/drcBasicTests.cc | 2 ++ src/drc/unit_tests/drcGenericTests.cc | 5 +++- src/drc/unit_tests/drcSimpleTests.cc | 31 +++++++++++++++++++++- src/drc/unit_tests/drcSuiteTests.cc | 5 +++- 7 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/buddies/src/bd/strmxor.cc b/src/buddies/src/bd/strmxor.cc index 1e5cf7722..6bed6fa16 100644 --- a/src/buddies/src/bd/strmxor.cc +++ b/src/buddies/src/bd/strmxor.cc @@ -707,7 +707,6 @@ bool run_deep_xor (const XORData &xor_data) { db::DeepShapeStore dss; dss.set_threads (xor_data.threads); - dss.set_keep_layouts (true); // avoids excessive cell mapping double dbu = std::min (xor_data.layout_a->dbu (), xor_data.layout_b->dbu ()); diff --git a/src/db/db/dbDeepShapeStore.cc b/src/db/db/dbDeepShapeStore.cc index ddda1ee2c..c22f1ba70 100644 --- a/src/db/db/dbDeepShapeStore.cc +++ b/src/db/db/dbDeepShapeStore.cc @@ -426,13 +426,13 @@ static unsigned int init_layer (db::Layout &layout, const db::RecursiveShapeIter } DeepShapeStore::DeepShapeStore () - : m_keep_layouts (false) + : m_keep_layouts (true) { ++s_instance_count; } DeepShapeStore::DeepShapeStore (const std::string &topcell_name, double dbu) - : m_keep_layouts (false) + : m_keep_layouts (true) { ++s_instance_count; diff --git a/src/drc/drc/built-in-macros/_drc_engine.rb b/src/drc/drc/built-in-macros/_drc_engine.rb index 7dce0a451..321d3ee79 100644 --- a/src/drc/drc/built-in-macros/_drc_engine.rb +++ b/src/drc/drc/built-in-macros/_drc_engine.rb @@ -29,6 +29,8 @@ module DRC cv = RBA::CellView::active + @time = Time::now + @force_gc = ($drc_force_gc == true) # for testing, $drc_force_gc can be set to true @generator = "" @rdb_index = nil @l2ndb_index = nil @@ -73,6 +75,11 @@ module DRC end + # for testing + def force_gc(f) + @force_gc = f + end + def both DRCBothEdges::new end @@ -2125,7 +2132,10 @@ CODE t = RBA::Timer::new t.start self._process_events - GC.start # force a garbage collection before the operation to free unused memory + if @force_gc || Time::now - @time > 0.5 + GC.start # force a garbage collection before the operation to free unused memory + @time = Time::now + end res = yield t.stop diff --git a/src/drc/unit_tests/drcBasicTests.cc b/src/drc/unit_tests/drcBasicTests.cc index 3e2adf02d..ea8917a0b 100644 --- a/src/drc/unit_tests/drcBasicTests.cc +++ b/src/drc/unit_tests/drcBasicTests.cc @@ -36,6 +36,7 @@ TEST(1) lym::Macro drc; drc.set_text (tl::sprintf ( + "force_gc true\n" "source('%s', \"TOP\")\n" "target('%s', \"TOP\")\n" "l1 = input(1, 0)\n" @@ -70,6 +71,7 @@ TEST(2) { lym::Macro drc; drc.set_text ( + "force_gc true\n" "dbu 0.001\n" "def compare(a, b, ex)\n" " a = a.to_s\n" diff --git a/src/drc/unit_tests/drcGenericTests.cc b/src/drc/unit_tests/drcGenericTests.cc index 10eccd418..ca6ac8d5f 100644 --- a/src/drc/unit_tests/drcGenericTests.cc +++ b/src/drc/unit_tests/drcGenericTests.cc @@ -30,6 +30,8 @@ static void run_test (tl::TestBase *_this, const std::string &number, bool deep) { + std::string force_gc = "true"; + std::string rs = tl::testdata (); rs += "/drc/drcGenericTests_" + number + ".drc"; @@ -45,10 +47,11 @@ static void run_test (tl::TestBase *_this, const std::string &number, bool deep) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = %s\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" "$drc_test_deep = %s\n" - , input, output, deep ? "true" : "false") + , force_gc, input, output, deep ? "true" : "false") ); config.set_interpreter (lym::Macro::Ruby); EXPECT_EQ (config.run (), 0); diff --git a/src/drc/unit_tests/drcSimpleTests.cc b/src/drc/unit_tests/drcSimpleTests.cc index 741646c9e..de1bfd6de 100644 --- a/src/drc/unit_tests/drcSimpleTests.cc +++ b/src/drc/unit_tests/drcSimpleTests.cc @@ -42,6 +42,7 @@ TEST(1) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = nil\n" "$drc_test_target = '%s'\n" , output) @@ -82,6 +83,7 @@ TEST(2) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -122,6 +124,7 @@ TEST(3_Flat) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -162,6 +165,7 @@ TEST(4_Hierarchical) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -202,6 +206,7 @@ TEST(5_FlatAntenna) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -242,6 +247,7 @@ TEST(5_FlatAntennaIncremental) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -282,6 +288,7 @@ TEST(6_HierarchicalAntenna) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -322,6 +329,7 @@ TEST(7_AntennaWithDiodes) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -362,6 +370,7 @@ TEST(8_TextsAndPolygons) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -427,6 +436,7 @@ TEST(9_NetlistExtraction) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" "$drc_test_target_simplified = '%s'\n" @@ -471,6 +481,7 @@ TEST(10_NetlistExtractionFlat) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" "$drc_test_target_simplified = '%s'\n" @@ -515,6 +526,7 @@ TEST(11_CustomDevices) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" "$drc_test_target_simplified = '%s'\n" @@ -555,6 +567,7 @@ TEST(12_NetlistJoinLabels) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" "$drc_test_target_simplified = nil\n" @@ -591,6 +604,7 @@ TEST(13a_KissingCorners) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -634,6 +648,7 @@ TEST(13b_KissingCornersDeep) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -681,6 +696,7 @@ TEST(14_SwitchingTargets) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" "$drc_test_target2 = '%s'\n" @@ -732,6 +748,7 @@ TEST(15_issue548) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -773,6 +790,7 @@ TEST(16_issue570) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -814,6 +832,7 @@ TEST(17_issue570) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -854,6 +873,7 @@ TEST(18_forget) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -894,6 +914,7 @@ TEST(19_shielding) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -934,6 +955,7 @@ TEST(20_interact_with_count) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -974,6 +996,7 @@ TEST(21_breaking) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -1014,6 +1037,7 @@ TEST(22_opposite_filter) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -1054,6 +1078,7 @@ TEST(23_rect_filter) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -1094,6 +1119,7 @@ TEST(24_enclosing) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" , input, output) @@ -1119,6 +1145,8 @@ TEST(24_enclosing) static void run_test (tl::TestBase *_this, const std::string &number, bool deep, bool oasis = false) { + std::string force_gc = "true"; + std::string rs = tl::testdata (); rs += "/drc/drcSimpleTests_" + number + ".drc"; @@ -1134,10 +1162,11 @@ static void run_test (tl::TestBase *_this, const std::string &number, bool deep, // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = '%s'\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" "$drc_test_deep = %s\n" - , input, output, deep ? "true" : "false") + , force_gc, input, output, deep ? "true" : "false") ); config.set_interpreter (lym::Macro::Ruby); EXPECT_EQ (config.run (), 0); diff --git a/src/drc/unit_tests/drcSuiteTests.cc b/src/drc/unit_tests/drcSuiteTests.cc index 3cfde1b3d..a329905e2 100644 --- a/src/drc/unit_tests/drcSuiteTests.cc +++ b/src/drc/unit_tests/drcSuiteTests.cc @@ -27,6 +27,8 @@ void runtest (tl::TestBase *_this, int mode) { + std::string force_gc = "true"; + std::string rs = tl::testdata (); rs += "/drc/drcSuiteTests.drc"; @@ -44,10 +46,11 @@ void runtest (tl::TestBase *_this, int mode) // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = %s\n" "$drc_test_source = '%s'\n" "$drc_test_target = '%s'\n" "$drc_test_mode = %d\n" - , input, output, mode) + , force_gc, input, output, mode) ); config.set_interpreter (lym::Macro::Ruby); EXPECT_EQ (config.run (), 0); From d8dcb41ee013c771f1a9a2a7c9434f6e1ce81257 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Thu, 24 Nov 2022 21:28:13 +0100 Subject: [PATCH 06/10] force GC on LVS tests too. --- src/lvs/unit_tests/lvsBasicTests.cc | 2 ++ src/lvs/unit_tests/lvsSimpleTests.cc | 1 + src/lvs/unit_tests/lvsTests.cc | 1 + 3 files changed, 4 insertions(+) diff --git a/src/lvs/unit_tests/lvsBasicTests.cc b/src/lvs/unit_tests/lvsBasicTests.cc index 1b8492d94..175f3bb51 100644 --- a/src/lvs/unit_tests/lvsBasicTests.cc +++ b/src/lvs/unit_tests/lvsBasicTests.cc @@ -40,6 +40,8 @@ TEST(1) lym::Macro lvs; lvs.set_text (tl::sprintf ( + "$drc_force_gc = true\n" + "\n" "source('%s', 'INVERTER')\n" "\n" "deep\n" diff --git a/src/lvs/unit_tests/lvsSimpleTests.cc b/src/lvs/unit_tests/lvsSimpleTests.cc index df95beedc..883eabf18 100644 --- a/src/lvs/unit_tests/lvsSimpleTests.cc +++ b/src/lvs/unit_tests/lvsSimpleTests.cc @@ -53,6 +53,7 @@ void run_test (tl::TestBase *_this, const std::string &suffix, const std::string // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$lvs_test_source = '%s'\n" "$lvs_test_target_lvsdb = '%s'\n" "$lvs_test_target_cir = '%s'\n" diff --git a/src/lvs/unit_tests/lvsTests.cc b/src/lvs/unit_tests/lvsTests.cc index 5475484bd..606eb26cd 100644 --- a/src/lvs/unit_tests/lvsTests.cc +++ b/src/lvs/unit_tests/lvsTests.cc @@ -47,6 +47,7 @@ void run_test (tl::TestBase *_this, const std::string &lvs_rs, const std::string // Set some variables lym::Macro config; config.set_text (tl::sprintf ( + "$drc_force_gc = true\n" "$lvs_test_source = '%s'\n" "$lvs_test_target_lvsdb = '%s'\n" "$lvs_test_target_cir = '%s'\n" From dc24ec2d156f728f7beb858b8c2bf85796387b4c Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Thu, 24 Nov 2022 21:46:56 +0100 Subject: [PATCH 07/10] Updated golden test data --- src/lvs/unit_tests/lvsTests.cc | 2 +- testdata/lvs/test_22c.lvsdb.1 | 24 ++++++++++++------------ testdata/lvs/test_22d.lvsdb.1 | 24 ++++++++++++------------ 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/lvs/unit_tests/lvsTests.cc b/src/lvs/unit_tests/lvsTests.cc index 606eb26cd..fcdbdecb3 100644 --- a/src/lvs/unit_tests/lvsTests.cc +++ b/src/lvs/unit_tests/lvsTests.cc @@ -142,7 +142,7 @@ TEST(16_private) TEST(17_private) { test_is_long_runner (); - run_test (_this, "test_17.lylvs", "test_17b.cir.gz", "test_17.gds.gz", true, "test_17b_2.lvsdb"); + run_test (_this, "test_17.lylvs", "test_17b.cir.gz", "test_17.gds.gz", true, "test_17b_3.lvsdb"); } TEST(18_private) diff --git a/testdata/lvs/test_22c.lvsdb.1 b/testdata/lvs/test_22c.lvsdb.1 index 6ee12b507..0e0b1750e 100644 --- a/testdata/lvs/test_22c.lvsdb.1 +++ b/testdata/lvs/test_22c.lvsdb.1 @@ -31,11 +31,11 @@ layout( layer(l25) layer(l26) layer(l27) - layer(l1) + layer(l22) layer(l2) layer(l4) layer(l6) - layer(l22) + layer(l1) # Mask layer connectivity connect(l5 l5 l4) @@ -54,21 +54,21 @@ layout( connect(l17 l18 l17 l26 l27) connect(l20 l20 l19) connect(l19 l20 l19 l27) - connect(l21 l9 l21 l2 l22) + connect(l21 l9 l21 l22 l2) connect(l23 l9 l11 l23) connect(l24 l11 l13 l24) connect(l25 l13 l15 l25) connect(l26 l15 l17 l26) connect(l27 l17 l19 l27) - connect(l1 l1) + connect(l22 l21 l22) connect(l2 l3 l21 l2 l4 l6) connect(l4 l5 l2 l4) connect(l6 l2 l6) - connect(l22 l21 l22) + connect(l1 l1) # Global nets and connectivity - global(l1 vss) global(l6 vss) + global(l1 vss) # Device class section class(active_res RES) @@ -193,22 +193,22 @@ layout( rect(l21 (-170 80) (170 170)) rect(l21 (-170 1070) (170 170)) rect(l21 (-5 -1010) (170 170)) - polygon(l2 (-465 -1120) (0 340) (-105 0) (0 420) (525 0) (0 -760)) - rect(l2 (-525 1670) (445 420)) - rect(l22 (-125 -1190) (330 270)) + rect(l22 (-250 -220) (330 270)) rect(l22 (950 -960) (150 2010)) rect(l22 (-1100 -1320) (950 150)) + polygon(l2 (-1495 -1050) (0 340) (-105 0) (0 420) (525 0) (0 -760)) + rect(l2 (-525 1670) (445 420)) ) net(4 polygon(l9 (1485 760) (0 840) (-245 0) (0 170) (245 0) (0 560) (170 0) (0 -1570)) rect(l21 (-170 80) (170 170)) rect(l21 (-170 1070) (170 170)) rect(l21 (-335 -650) (170 170)) - polygon(l2 (-125 -1480) (0 760) (525 0) (0 -420) (-105 0) (0 -340)) - rect(l2 (-340 1670) (445 420)) - rect(l22 (-650 -830) (330 270)) + rect(l22 (-250 -220) (330 270)) rect(l22 (-1280 -150) (950 150)) rect(l22 (-1100 -1320) (150 2010)) + polygon(l2 (1075 -2220) (0 760) (525 0) (0 -420) (-105 0) (0 -340)) + rect(l2 (-340 1670) (445 420)) ) net(5 name(vdd) rect(l5 (-385 1780) (2950 1300)) diff --git a/testdata/lvs/test_22d.lvsdb.1 b/testdata/lvs/test_22d.lvsdb.1 index 3c52f6e73..4f171845c 100644 --- a/testdata/lvs/test_22d.lvsdb.1 +++ b/testdata/lvs/test_22d.lvsdb.1 @@ -31,11 +31,11 @@ layout( layer(l25) layer(l26) layer(l27) - layer(l1) + layer(l22) layer(l2) layer(l4) layer(l6) - layer(l22) + layer(l1) # Mask layer connectivity connect(l5 l5 l4) @@ -54,21 +54,21 @@ layout( connect(l17 l18 l17 l26 l27) connect(l20 l20 l19) connect(l19 l20 l19 l27) - connect(l21 l9 l21 l2 l22) + connect(l21 l9 l21 l22 l2) connect(l23 l9 l11 l23) connect(l24 l11 l13 l24) connect(l25 l13 l15 l25) connect(l26 l15 l17 l26) connect(l27 l17 l19 l27) - connect(l1 l1) + connect(l22 l21 l22) connect(l2 l3 l21 l2 l4 l6) connect(l4 l5 l2 l4) connect(l6 l2 l6) - connect(l22 l21 l22) + connect(l1 l1) # Global nets and connectivity - global(l1 vss) global(l6 vss) + global(l1 vss) # Device class section class(active_res RES) @@ -193,22 +193,22 @@ layout( rect(l21 (-170 80) (170 170)) rect(l21 (-170 1070) (170 170)) rect(l21 (-5 -1010) (170 170)) - polygon(l2 (-465 -1120) (0 340) (-105 0) (0 420) (525 0) (0 -760)) - rect(l2 (-525 1670) (445 420)) - rect(l22 (-125 -1190) (330 270)) + rect(l22 (-250 -220) (330 270)) rect(l22 (950 -960) (150 2010)) rect(l22 (-1100 -1320) (950 150)) + polygon(l2 (-1495 -1050) (0 340) (-105 0) (0 420) (525 0) (0 -760)) + rect(l2 (-525 1670) (445 420)) ) net(4 polygon(l9 (1485 760) (0 840) (-245 0) (0 170) (245 0) (0 560) (170 0) (0 -1570)) rect(l21 (-170 80) (170 170)) rect(l21 (-170 1070) (170 170)) rect(l21 (-335 -650) (170 170)) - polygon(l2 (-125 -1480) (0 760) (525 0) (0 -420) (-105 0) (0 -340)) - rect(l2 (-340 1670) (445 420)) - rect(l22 (-650 -830) (330 270)) + rect(l22 (-250 -220) (330 270)) rect(l22 (-1280 -150) (950 150)) rect(l22 (-1100 -1320) (150 2010)) + polygon(l2 (1075 -2220) (0 760) (525 0) (0 -420) (-105 0) (0 -340)) + rect(l2 (-340 1670) (445 420)) ) net(5 name(vdd) rect(l5 (-385 1780) (2950 1300)) From 66b10245fe92df249f165fdacd9a84216374a498 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Thu, 24 Nov 2022 21:54:04 +0100 Subject: [PATCH 08/10] Another update of golden data --- src/lvs/unit_tests/lvsTests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lvs/unit_tests/lvsTests.cc b/src/lvs/unit_tests/lvsTests.cc index fcdbdecb3..989124420 100644 --- a/src/lvs/unit_tests/lvsTests.cc +++ b/src/lvs/unit_tests/lvsTests.cc @@ -160,7 +160,7 @@ TEST(19_private) TEST(20_private) { // test_is_long_runner (); - run_test (_this, "test_20.lylvs", "test_20.cir.gz", "test_20.gds.gz", true, "test_20b_2.lvsdb"); + run_test (_this, "test_20.lylvs", "test_20.cir.gz", "test_20.gds.gz", true, "test_20b_3.lvsdb"); } TEST(21_private) From 20b66084104c114e505abfc2d8de3f773080e4d6 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Thu, 24 Nov 2022 22:12:01 +0100 Subject: [PATCH 09/10] Test updated --- src/db/unit_tests/dbDeepShapeStoreTests.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/db/unit_tests/dbDeepShapeStoreTests.cc b/src/db/unit_tests/dbDeepShapeStoreTests.cc index c629f2c6b..223ca3ac1 100644 --- a/src/db/unit_tests/dbDeepShapeStoreTests.cc +++ b/src/db/unit_tests/dbDeepShapeStoreTests.cc @@ -73,6 +73,7 @@ static size_t shapes_in_top (const db::Layout &layout, unsigned int layer) TEST(2_RefCounting) { db::DeepShapeStore store; + store.set_keep_layouts (false); db::Layout layout; unsigned int l1 = layout.insert_layer (); From 50ec2ff0065a2d3bea052d58ddefed0f2c8bec01 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Fri, 25 Nov 2022 00:12:31 +0100 Subject: [PATCH 10/10] Updating golden test data --- testdata/lvs/test_22c.lvsdb.2 | 24 ++++++++++++------------ testdata/lvs/test_22d.lvsdb.2 | 24 ++++++++++++------------ 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/testdata/lvs/test_22c.lvsdb.2 b/testdata/lvs/test_22c.lvsdb.2 index a3bacb5a7..4e5c37050 100644 --- a/testdata/lvs/test_22c.lvsdb.2 +++ b/testdata/lvs/test_22c.lvsdb.2 @@ -31,11 +31,11 @@ layout( layer(l25) layer(l26) layer(l27) - layer(l1) + layer(l22) layer(l2) layer(l4) layer(l6) - layer(l22) + layer(l1) # Mask layer connectivity connect(l5 l5 l4) @@ -54,21 +54,21 @@ layout( connect(l17 l18 l17 l26 l27) connect(l20 l20 l19) connect(l19 l20 l19 l27) - connect(l21 l9 l21 l2 l22) + connect(l21 l9 l21 l22 l2) connect(l23 l9 l11 l23) connect(l24 l11 l13 l24) connect(l25 l13 l15 l25) connect(l26 l15 l17 l26) connect(l27 l17 l19 l27) - connect(l1 l1) + connect(l22 l21 l22) connect(l2 l3 l21 l2 l4 l6) connect(l4 l5 l2 l4) connect(l6 l2 l6) - connect(l22 l21 l22) + connect(l1 l1) # Global nets and connectivity - global(l1 vss) global(l6 vss) + global(l1 vss) # Device class section class(active_res RES) @@ -193,22 +193,22 @@ layout( rect(l21 (-170 80) (170 170)) rect(l21 (-170 1070) (170 170)) rect(l21 (-5 -1010) (170 170)) - polygon(l2 (-465 -1120) (0 340) (-105 0) (0 420) (525 0) (0 -760)) - rect(l2 (-525 1670) (445 420)) - rect(l22 (-125 -1190) (330 270)) + rect(l22 (-250 -220) (330 270)) rect(l22 (950 -960) (150 2010)) rect(l22 (-1100 -1320) (950 150)) + polygon(l2 (-1495 -1050) (0 340) (-105 0) (0 420) (525 0) (0 -760)) + rect(l2 (-525 1670) (445 420)) ) net(4 polygon(l9 (1485 760) (0 840) (-245 0) (0 170) (245 0) (0 560) (170 0) (0 -1570)) rect(l21 (-170 80) (170 170)) rect(l21 (-170 1070) (170 170)) rect(l21 (-335 -650) (170 170)) - polygon(l2 (-125 -1480) (0 760) (525 0) (0 -420) (-105 0) (0 -340)) - rect(l2 (-340 1670) (445 420)) - rect(l22 (-650 -830) (330 270)) + rect(l22 (-250 -220) (330 270)) rect(l22 (-1280 -150) (950 150)) rect(l22 (-1100 -1320) (150 2010)) + polygon(l2 (1075 -2220) (0 760) (525 0) (0 -420) (-105 0) (0 -340)) + rect(l2 (-340 1670) (445 420)) ) net(5 name(vdd) rect(l5 (-385 1780) (2950 1300)) diff --git a/testdata/lvs/test_22d.lvsdb.2 b/testdata/lvs/test_22d.lvsdb.2 index 24c24136c..3a3bd09c8 100644 --- a/testdata/lvs/test_22d.lvsdb.2 +++ b/testdata/lvs/test_22d.lvsdb.2 @@ -31,11 +31,11 @@ layout( layer(l25) layer(l26) layer(l27) - layer(l1) + layer(l22) layer(l2) layer(l4) layer(l6) - layer(l22) + layer(l1) # Mask layer connectivity connect(l5 l5 l4) @@ -54,21 +54,21 @@ layout( connect(l17 l18 l17 l26 l27) connect(l20 l20 l19) connect(l19 l20 l19 l27) - connect(l21 l9 l21 l2 l22) + connect(l21 l9 l21 l22 l2) connect(l23 l9 l11 l23) connect(l24 l11 l13 l24) connect(l25 l13 l15 l25) connect(l26 l15 l17 l26) connect(l27 l17 l19 l27) - connect(l1 l1) + connect(l22 l21 l22) connect(l2 l3 l21 l2 l4 l6) connect(l4 l5 l2 l4) connect(l6 l2 l6) - connect(l22 l21 l22) + connect(l1 l1) # Global nets and connectivity - global(l1 vss) global(l6 vss) + global(l1 vss) # Device class section class(active_res RES) @@ -193,22 +193,22 @@ layout( rect(l21 (-170 80) (170 170)) rect(l21 (-170 1070) (170 170)) rect(l21 (-5 -1010) (170 170)) - polygon(l2 (-465 -1120) (0 340) (-105 0) (0 420) (525 0) (0 -760)) - rect(l2 (-525 1670) (445 420)) - rect(l22 (-125 -1190) (330 270)) + rect(l22 (-250 -220) (330 270)) rect(l22 (950 -960) (150 2010)) rect(l22 (-1100 -1320) (950 150)) + polygon(l2 (-1495 -1050) (0 340) (-105 0) (0 420) (525 0) (0 -760)) + rect(l2 (-525 1670) (445 420)) ) net(4 polygon(l9 (1485 760) (0 840) (-245 0) (0 170) (245 0) (0 560) (170 0) (0 -1570)) rect(l21 (-170 80) (170 170)) rect(l21 (-170 1070) (170 170)) rect(l21 (-335 -650) (170 170)) - polygon(l2 (-125 -1480) (0 760) (525 0) (0 -420) (-105 0) (0 -340)) - rect(l2 (-340 1670) (445 420)) - rect(l22 (-650 -830) (330 270)) + rect(l22 (-250 -220) (330 270)) rect(l22 (-1280 -150) (950 150)) rect(l22 (-1100 -1320) (150 2010)) + polygon(l2 (1075 -2220) (0 760) (525 0) (0 -420) (-105 0) (0 -340)) + rect(l2 (-340 1670) (445 420)) ) net(5 name(vdd) rect(l5 (-385 1780) (2950 1300))