From 403840b10938fdfb6d0063d9253cb06398adf13f Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Fri, 1 Jun 2018 23:36:01 +0200 Subject: [PATCH] WIP: some bug and performance fixes. --- src/gsi/gsi/gsi.cc | 10 ----- src/gsi/gsi/gsiClassBase.cc | 44 ++++++++++++++++++--- src/gsi/gsi/gsiClassBase.h | 7 ++++ src/pya/pya/pya.cc | 79 ++++++++++++++++++++++++++----------- 4 files changed, 101 insertions(+), 39 deletions(-) diff --git a/src/gsi/gsi/gsi.cc b/src/gsi/gsi/gsi.cc index 208a5c2f6..b8904c67f 100644 --- a/src/gsi/gsi/gsi.cc +++ b/src/gsi/gsi/gsi.cc @@ -51,16 +51,6 @@ initialize () // merge the extensions to the main declaration gsi::ClassBase::merge_declarations (); - // do a full re-initialization - maybe merge_declarations modified existing classes too - for (gsi::ClassBase::class_iterator c = gsi::ClassBase::begin_classes (); c != gsi::ClassBase::end_classes (); ++c) { - // Initialize the method table once again after we have merged the declarations - // TODO: get rid of that const cast - (const_cast (&*c))->initialize (); - - // there should be only main declarations since we merged - tl_assert (c->declaration () == &*c); - } - // build or rebuild the variant user class table // NOTE: as the variant classes are tied to the gsi::Class objects, we can rebuild the table // and will get the same pointers for the classes that have been there before diff --git a/src/gsi/gsi/gsiClassBase.cc b/src/gsi/gsi/gsiClassBase.cc index f2b8baaa2..617ac0658 100644 --- a/src/gsi/gsi/gsiClassBase.cc +++ b/src/gsi/gsi/gsiClassBase.cc @@ -55,7 +55,7 @@ namespace { static std::map s_ti_to_class; ClassBase::ClassBase (const std::string &doc, const Methods &mm, bool do_register) - : mp_base (0), mp_parent (0), m_doc (doc), m_methods (mm) + : m_initialized (false), mp_base (0), mp_parent (0), m_doc (doc), m_methods (mm) { if (do_register) { @@ -79,13 +79,19 @@ ClassBase::~ClassBase () void ClassBase::set_parent (const ClassBase *p) { - mp_parent = p; + if (mp_parent != p) { + mp_parent = p; + m_initialized = false; + } } void ClassBase::set_base (const ClassBase *b) { - mp_base = b; + if (mp_base != b) { + mp_base = b; + m_initialized = false; + } } std::string @@ -100,6 +106,15 @@ ClassBase::qname () const return qn; } +void +ClassBase::add_subclass (const ClassBase *cls) +{ + // TODO: ugly const_cast hack + ClassBase *non_const_cls = const_cast (cls); + m_subclasses.push_back (non_const_cls); + m_initialized = false; +} + void ClassBase::add_child_class (const ClassBase *cls) { @@ -109,6 +124,7 @@ ClassBase::add_child_class (const ClassBase *cls) // child classes inherit the module of their parent non_const_cls->set_module (module ()); m_child_classes.push_back (non_const_cls); + m_initialized = false; } bool @@ -395,7 +411,7 @@ ClassBase::merge_declarations () // Consolidate the classes (merge, remove etc.) for (gsi::ClassBase::class_iterator c = gsi::ClassBase::begin_new_classes (); c != gsi::ClassBase::end_new_classes (); ++c) { - if (! c->consolidate()) { + if (! c->consolidate ()) { to_remove.push_back (&*c); } } @@ -410,7 +426,7 @@ ClassBase::merge_declarations () for (gsi::ClassBase::class_iterator c = gsi::ClassBase::begin_new_classes (); c != gsi::ClassBase::end_new_classes (); ++c) { if (c->base ()) { // TODO: ugly const_cast hack - const_cast (c->base ())->m_subclasses.push_back (const_cast (c.operator-> ())); + const_cast (c->base ())->add_subclass (c.operator-> ()); } } @@ -499,11 +515,26 @@ ClassBase::merge_declarations () mp_class_collection->push_back (non_const_decl); } mp_new_class_collection->clear (); + + // do a full re-initialization - maybe merge_declarations modified existing classes too + for (gsi::ClassBase::class_iterator c = gsi::ClassBase::begin_classes (); c != gsi::ClassBase::end_classes (); ++c) { + // Initialize the method table once again after we have merged the declarations + // TODO: get rid of that const cast + (const_cast (&*c))->initialize (); + + // there should be only main declarations since we merged + tl_assert (c->declaration () == &*c); + } } void ClassBase::initialize () { + // don't initialize again + if (m_initialized) { + return; + } + m_methods.initialize (); m_constructors.clear (); @@ -519,11 +550,14 @@ ClassBase::initialize () m_callbacks.push_back (*m); } } + + m_initialized = true; } void ClassBase::add_method (MethodBase *method, bool /*base_class*/) { + m_initialized = false; m_methods.add_method (method); } diff --git a/src/gsi/gsi/gsiClassBase.h b/src/gsi/gsi/gsiClassBase.h index 544475a91..92b3a3f75 100644 --- a/src/gsi/gsi/gsiClassBase.h +++ b/src/gsi/gsi/gsiClassBase.h @@ -188,6 +188,12 @@ public: */ void add_child_class (const ClassBase *cls); + /** + * @brief Adds a subclass + * Subclasses are the ones that derive from us (opposite of base) + */ + void add_subclass (const ClassBase *cls); + /** * @brief Iterates all child classes (begin) */ @@ -595,6 +601,7 @@ protected: void set_base (const ClassBase *base); private: + bool m_initialized; const ClassBase *mp_base, *mp_parent; std::string m_doc; Methods m_methods; diff --git a/src/pya/pya/pya.cc b/src/pya/pya/pya.cc index 625452425..4f76e58c3 100644 --- a/src/pya/pya/pya.cc +++ b/src/pya/pya/pya.cc @@ -2223,21 +2223,7 @@ PythonModule::PythonModule () PythonModule::~PythonModule () { PYAObjectBase::clear_callbacks_cache (); - - while (!m_methods_heap.empty ()) { - delete m_methods_heap.back (); - m_methods_heap.pop_back (); - } - - while (!m_getseters_heap.empty ()) { - delete m_getseters_heap.back (); - m_getseters_heap.pop_back (); - } - - if (mp_mod_def) { - delete[] mp_mod_def; - mp_mod_def = 0; - } + delete_module (); } PyObject * @@ -2256,6 +2242,22 @@ void PythonModule::delete_module () { mp_module = PythonRef (); + mp_base_class = PythonRef (); + + while (!m_methods_heap.empty ()) { + delete m_methods_heap.back (); + m_methods_heap.pop_back (); + } + + while (!m_getseters_heap.empty ()) { + delete m_getseters_heap.back (); + m_getseters_heap.pop_back (); + } + + if (mp_mod_def) { + delete[] mp_mod_def; + mp_mod_def = 0; + } } void @@ -2283,12 +2285,13 @@ PythonModule::init (const char *mod_name, const char *description) module_methods }; - tl_assert (mp_mod_def == 0); + tl_assert (! mp_mod_def); // prepare a persistent structure with the module definition // and pass this one to PyModule_Create mp_mod_def = new char[sizeof (PyModuleDef)]; memcpy ((void *) mp_mod_def, (const void *) &mod_def, sizeof (PyModuleDef)); + module = PyModule_Create ((PyModuleDef *) mp_mod_def); #endif @@ -2350,6 +2353,39 @@ PythonModule::python_doc (const gsi::MethodBase *method) void PythonModule::make_classes (const char *mod_name) { + // Check whether the new classes are self-contained within this module + if (mod_name) { + + for (gsi::ClassBase::class_iterator c = gsi::ClassBase::begin_classes (); c != gsi::ClassBase::end_classes (); ++c) { + + if (c->module () != mod_name) { + // don't handle classes outside this module + continue; + } + + if (m_rev_cls_map.find (&*c) != m_rev_cls_map.end ()) { + // don't handle classes twice + continue; + } + + // All child classes must originate from this module or be known already + for (tl::weak_collection::const_iterator cc = c->begin_child_classes (); cc != c->end_child_classes (); ++cc) { + if (m_rev_cls_map.find (cc->declaration ()) == m_rev_cls_map.end () + && cc->module () != mod_name) { + throw tl::Exception (tl::sprintf (tl::to_string (QObject::tr ("Class %s from module %s depends on %s.%s (try 'import klayout.%s' before 'import klayout.%s')")), c->name (), mod_name, cc->module (), cc->name (), cc->module (), mod_name)); + } + } + + // Same for base class + if (c->base () && m_rev_cls_map.find (c->base ()) == m_rev_cls_map.end () + && c->base ()->module () != mod_name) { + throw tl::Exception (tl::sprintf (tl::to_string (QObject::tr ("Class %s from module %s depends on %s.%s (try 'import klayout.%s' before 'import klayout.%s')")), c->name (), mod_name, c->base ()->module (), c->base ()->name (), c->base ()->module (), mod_name)); + } + + } + + } + PyObject *module = mp_module.get (); // Prepare an __all__ index for the module @@ -2426,9 +2462,6 @@ PythonModule::make_classes (const char *mod_name) for (tl::weak_collection::const_iterator cc = c->begin_child_classes (); cc != c->end_child_classes (); ++cc) { tl_assert (cc->declaration () != 0); if (m_rev_cls_map.find (cc->declaration ()) == m_rev_cls_map.end ()) { - if (mod_name && cc->module () != mod_name) { - throw tl::Exception (tl::sprintf (tl::to_string (QObject::tr ("Class %s from module %s depends on %s.%s (try 'import klayout.%s' before 'import klayout.%s')")), c->name (), mod_name, cc->module (), cc->name (), cc->module (), mod_name)); - } all_children_available = false; break; } @@ -2440,9 +2473,6 @@ PythonModule::make_classes (const char *mod_name) } if (c->base () && m_rev_cls_map.find (c->base ()) == m_rev_cls_map.end ()) { - if (mod_name && c->base ()->module () != mod_name) { - throw tl::Exception (tl::sprintf (tl::to_string (QObject::tr ("Class %s from module %s depends on %s.%s (try 'import klayout.%s' before 'import klayout.%s')")), c->name (), mod_name, c->base ()->module (), c->base ()->name (), c->base ()->module (), mod_name)); - } // can't produce this class yet. The base class needs to be handled first. more_classes = true; continue; @@ -2478,8 +2508,9 @@ PythonModule::make_classes (const char *mod_name) PyList_Append (all_list.get (), PythonRef (c2python (c->name ())).get ()); PyModule_AddObject (module, c->name ().c_str (), (PyObject *) type); - m_cls_map.insert (std::make_pair (type, &*c)); - m_rev_cls_map.insert (std::make_pair (&*c, type)); + // NOTE: we force the new values as there may be value from a previous (failed) attempt + m_cls_map[type] = c.operator-> (); + m_rev_cls_map[c.operator-> ()] = type; // Create the sub-class attributes