[consider merging] Fixed a MT problem that can be fixed by avoiding recursive calls of Layout::update

src/db/db/dbBoxTree.h

@@ -981,9 +981,10 @@ public:
    *  Only after sorting the query iterators are available.
    *  Sorting complexity is approx O(N*log(N)).
    */
-  void sort (const BoxConv &conv)
+  template <class BC>
+  void sort (const BC &conv)
   {
-    typename BoxConv::complexity complexity_tag;
+    typename BC::complexity complexity_tag;
     sort (conv, complexity_tag);
   }
 
@@ -1192,7 +1193,8 @@ private:
   box_tree_node *mp_root;
 
   /// Sort implementation for simple bboxes - no caching
-  void sort (const BoxConv &conv, const db::simple_bbox_tag &/*complexity*/)
+  template <class BC>
+  void sort (const BC &conv, const db::simple_bbox_tag &/*complexity*/)
   {
     m_elements.clear ();
     m_elements.reserve (m_objects.size ());
@@ -1204,7 +1206,7 @@ private:
 
     if (! m_objects.empty ()) {
 
-      box_tree_picker_type picker (conv);
+      box_tree_picker<Box, Obj, BC, obj_vector_type> picker (conv);
 
       box_type bbox;
       for (typename obj_vector_type::const_iterator o = m_objects.begin (); o != m_objects.end (); ++o) {
@@ -1221,7 +1223,8 @@ private:
   }
 
   /// Sort implementation for complex bboxes - with caching
-  void sort (const box_conv_type &conv, const db::complex_bbox_tag &/*complexity*/)
+  template <class BC>
+  void sort (const BC &conv, const db::complex_bbox_tag &/*complexity*/)
   {
     m_elements.clear ();
     m_elements.reserve (m_objects.size ());
@@ -1233,7 +1236,7 @@ private:
 
     if (! m_objects.empty ()) {
 
-      box_tree_cached_picker<object_type, box_type, box_conv_type, obj_vector_type> picker (conv, m_objects.begin (), m_objects.end ());
+      box_tree_cached_picker<object_type, box_type, BC, obj_vector_type> picker (conv, m_objects.begin (), m_objects.end ());
 
       for (typename obj_vector_type::const_iterator o = m_objects.begin (); o != m_objects.end (); ++o) {
         m_elements.push_back (o.index ());
@@ -1997,9 +2000,10 @@ public:
    *  Only after sorting the query iterators are available.
    *  Sorting complexity is approx O(N*log(N)).
    */
-  void sort (const BoxConv &conv)
+  template <class BC>
+  void sort (const BC &conv)
   {
-    typename BoxConv::complexity complexity_tag;
+    typename BC::complexity complexity_tag;
     sort (conv, complexity_tag);
   }
 
@@ -2159,13 +2163,14 @@ private:
   box_tree_node *mp_root;
 
   /// Sort implementation for simple bboxes - no caching
-  void sort (const BoxConv &conv, const db::simple_bbox_tag &/*complexity*/)
+  template <class BC>
+  void sort (const BC &conv, const db::simple_bbox_tag &/*complexity*/)
   {
     if (m_objects.empty ()) {
       return;
     }
 
-    box_tree_picker_type picker (conv);
+    box_tree_picker<box_type, object_type, BC, obj_vector_type> picker (conv);
 
     if (mp_root) {
       delete mp_root;
@@ -2184,13 +2189,14 @@ private:
   }
 
   /// Sort implementation for complex bboxes - with caching
-  void sort (const box_conv_type &conv, const db::complex_bbox_tag &/*complexity*/)
+  template <class BC>
+  void sort (const BC &conv, const db::complex_bbox_tag &/*complexity*/)
   {
     if (m_objects.empty ()) {
       return;
     }
 
-    box_tree_cached_picker<object_type, box_type, box_conv_type, obj_vector_type> picker (conv, m_objects.begin (), m_objects.end ());
+    box_tree_cached_picker<object_type, box_type, BC, obj_vector_type> picker (conv, m_objects.begin (), m_objects.end ());
 
     if (mp_root) {
       delete mp_root;

src/db/db/dbCell.cc

@@ -274,11 +274,67 @@ Cell::is_shape_bbox_dirty () const
   return false;
 }
 
+namespace
+{
+
+/**
+ *  @brief An alternative box converter for CellInst which does not call layout.update
+ *
+ *  Using this converter in cell.update_bbox() prevents recursive calls to layout.update.
+ */
+class InternalCellInstBoxConverter
+{
+public:
+  typedef db::Cell::box_type box_type;
+  typedef db::simple_bbox_tag complexity;
+
+  InternalCellInstBoxConverter (const db::Layout *layout)
+    : mp_layout (layout)
+  {
+    //  .. nothing yet ..
+  }
+
+  box_type operator() (const db::CellInst &cell_inst) const
+  {
+    return mp_layout->cell (cell_inst.cell_index ()).bbox_with_empty_no_update ();
+  }
+
+private:
+  const db::Layout *mp_layout;
+};
+
+/**
+ *  @brief An alternative box converter for CellInst and layer which does not call layout.update
+ *
+ *  Using this converter in cell.update_bbox() prevents recursive calls to layout.update.
+ */
+class InternalPerLayerCellInstBoxConverter
+{
+public:
+  typedef db::Cell::box_type box_type;
+  typedef db::simple_bbox_tag complexity;
+
+  InternalPerLayerCellInstBoxConverter (const db::Layout *layout, unsigned int layer)
+    : mp_layout (layout), m_layer (layer)
+  {
+    //  .. nothing yet ..
+  }
+
+  box_type operator() (const db::CellInst &cell_inst) const
+  {
+    return mp_layout->cell (cell_inst.cell_index ()).bbox_no_update (m_layer);
+  }
+
+private:
+  const db::Layout *mp_layout;
+  unsigned int m_layer;
+};
+
+}
+
 bool 
 Cell::update_bbox (unsigned int layers)
 {
-  unsigned int l;
-
   //  determine the bounding box
   box_type org_bbox = m_bbox;
   m_bbox = box_type ();
@@ -308,10 +364,10 @@ Cell::update_bbox (unsigned int layers)
       ++o;
     }
 
-    for (l = 0; l < layers; ++l) {
+    for (unsigned int l = 0; l < layers; ++l) {
 
       //  the per-layer bounding boxes
-      db::box_convert <cell_inst_type> bc (*mp_layout, l);
+      InternalPerLayerCellInstBoxConverter bc (mp_layout, l);
       box_type lbox = o1_inst->bbox_from_raw_bbox (raw_box, bc);
 
       if (! lbox.empty ()) {
@@ -326,7 +382,7 @@ Cell::update_bbox (unsigned int layers)
 
     }
 
-    db::box_convert <cell_inst_type, false> bc_we (*mp_layout);
+    InternalCellInstBoxConverter bc_we (mp_layout);
     m_bbox_with_empty += o1_inst->bbox_from_raw_bbox (raw_box, bc_we);
     
   }
@@ -486,6 +542,17 @@ Cell::bbox (unsigned int l) const
   }
 }
 
+const Cell::box_type &
+Cell::bbox_no_update (unsigned int l) const
+{
+  box_map::const_iterator b = m_bboxes.find (l);
+  if (b != m_bboxes.end ()) {
+    return b->second;
+  } else {
+    return ms_empty_box;
+  }
+}
+
 Cell::const_iterator
 Cell::begin () const
 {
@@ -725,7 +792,7 @@ Cell::count_hier_levels () const
 {
   unsigned int l = 0;
 
-  for (const_iterator c = begin (); !c.at_end (); ++c) {
+  for (const_iterator c = m_instances.begin (); !c.at_end (); ++c) {
     l = std::max (l, (unsigned int) mp_layout->cell (c->cell_index ()).m_hier_levels + 1);
   }
 

src/db/db/dbCell.h

@@ -522,21 +522,6 @@ public:
    */
   bool is_shape_bbox_dirty () const;
 
-  /** 
-   *  @brief Updates the bbox
-   *
-   *  This will update the bbox from the shapes and instances.
-   *  This requires the bboxes of the child cells to be computed
-   *  before. Practically this will be done by computing the
-   *  bboxes bottom-up in the hierarchy.
-   *  In addition, the number of hierarchy levels below is also
-   *  updated.
-   *
-   *  @param layers The max. number of layers in the child cells
-   *  @return true, if the bounding box has changed.
-   */
-  bool update_bbox (unsigned int layers);
-
   /**
    *  @brief Sorts the shapes lists
    *
@@ -1110,6 +1095,23 @@ public:
    */
   void move_shapes (db::Cell &source_cell, const db::LayerMapping &layer_mapping);
 
+  /**
+   *  @brief Gets the current bounding box (with empty) without calling Layout::update
+   *
+   *  This method is intended for internal purposes only.
+   */
+  const box_type &bbox_with_empty_no_update () const
+  {
+    return m_bbox_with_empty;
+  }
+
+  /**
+   *  @brief Gets the current per-layer bounding box without calling Layout::update
+   *
+   *  This method is intended for internal purposes only.
+   */
+  const box_type &bbox_no_update (unsigned int l) const;
+
 protected:
   /**
    *  @brief Standard constructor: create an empty cell object
@@ -1135,6 +1137,7 @@ protected:
   virtual Cell *clone (db::Layout &layout) const;
 
 private:
+  friend class db::Layout;
   cell_index_type m_cell_index;
   mutable db::Layout *mp_layout;
   shapes_map m_shapes_map;
@@ -1172,7 +1175,7 @@ private:
   /**
    *  @brief Return a reference to the instances object
    */
-  instances_type &instances ()
+  const instances_type &instances () const
   {
     return m_instances;
   }
@@ -1180,7 +1183,7 @@ private:
   /**
    *  @brief Return a reference to the instances object 
    */
-  const instances_type &instances () const
+  instances_type &instances ()
   {
     return m_instances;
   }
@@ -1219,6 +1222,21 @@ private:
    *  @param force Force sorting, even if not strictly needed
    */
   void sort_inst_tree (bool force);
+
+  /**
+   *  @brief Updates the bbox
+   *
+   *  This will update the bbox from the shapes and instances.
+   *  This requires the bboxes of the child cells to be computed
+   *  before. Practically this will be done by computing the
+   *  bboxes bottom-up in the hierarchy.
+   *  In addition, the number of hierarchy levels below is also
+   *  updated.
+   *
+   *  @param layers The max. number of layers in the child cells
+   *  @return true, if the bounding box has changed.
+   */
+  bool update_bbox (unsigned int layers);
 };
 
 /**

src/db/db/dbInstances.cc

@@ -1524,6 +1524,84 @@ Instances::sort_child_insts (bool force)
   std::sort (m_insts_by_cell_index.begin (), m_insts_by_cell_index.end (), cell_inst_compare_f<basic_inst_type> ());
 }
 
+namespace {
+
+/**
+ *  @brief An alternative box converter for CellInst which does not call layout.update
+ *
+ *  Using this converter in cell.update_bbox() prevents recursive calls to layout.update.
+ */
+class InternalCellInstBoxConverter
+{
+public:
+  typedef db::Cell::box_type box_type;
+  typedef db::simple_bbox_tag complexity;
+
+  InternalCellInstBoxConverter (const db::Layout *layout)
+    : mp_layout (layout)
+  {
+    //  .. nothing yet ..
+  }
+
+  db::Cell::box_type operator() (const db::Cell::cell_inst_type &cell_inst) const
+  {
+    return mp_layout->cell (cell_inst.cell_index ()).bbox_with_empty_no_update ();
+  }
+
+private:
+  const db::Layout *mp_layout;
+};
+
+/**
+ *  @brief An alternative box converter for CellInstArray with properties which does not call layout.update
+ *
+ *  Using this converter in cell.update_bbox() prevents recursive calls to layout.update.
+ */
+struct InternalCellInstArrayWithPropertiesBoxConverter
+{
+  typedef db::Cell::cell_inst_array_type cell_inst_array;
+  typedef db::Cell::box_type box_type;
+  typedef db::complex_bbox_tag complexity;
+
+  InternalCellInstArrayWithPropertiesBoxConverter (const db::Layout *layout)
+    : m_bc (layout)
+  { }
+
+  box_type operator() (const db::object_with_properties<cell_inst_array> &array) const
+  {
+    return array.bbox (m_bc);
+  }
+
+private:
+  InternalCellInstBoxConverter m_bc;
+};
+
+/**
+ *  @brief An alternative box converter for CellInstArray which does not call layout.update
+ *
+ *  Using this converter in cell.update_bbox() prevents recursive calls to layout.update.
+ */
+struct InternalCellInstArrayBoxConverter
+{
+  typedef db::Cell::cell_inst_array_type cell_inst_array;
+  typedef db::Cell::box_type box_type;
+  typedef db::complex_bbox_tag complexity;
+
+  InternalCellInstArrayBoxConverter (const db::Layout *layout)
+    : m_bc (layout)
+  { }
+
+  box_type operator() (const cell_inst_array &array) const
+  {
+    return array.bbox (m_bc);
+  }
+
+private:
+  InternalCellInstBoxConverter m_bc;
+};
+
+}
+
 void 
 Instances::sort_inst_tree (const Layout *g, bool force)
 {
@@ -1534,18 +1612,18 @@ Instances::sort_inst_tree (const Layout *g, bool force)
 
   if (m_generic.any) {
     if (is_editable ()) {
-      m_generic.stable_tree->sort (cell_inst_array_box_converter (*g));
+      m_generic.stable_tree->sort (InternalCellInstArrayBoxConverter (g));
     } else {
-      m_generic.unstable_tree->sort (cell_inst_array_box_converter (*g));
+      m_generic.unstable_tree->sort (InternalCellInstArrayBoxConverter (g));
       //  since we use unstable instance trees in non-editable mode, we need to resort the child instances in this case
       sort_child_insts (true);
     }
   }
   if (m_generic_wp.any) {
     if (is_editable ()) {
-      m_generic_wp.stable_tree->sort (cell_inst_wp_array_box_converter (*g));
+      m_generic_wp.stable_tree->sort (InternalCellInstArrayWithPropertiesBoxConverter (g));
     } else {
-      m_generic_wp.unstable_tree->sort (cell_inst_wp_array_box_converter (*g));
+      m_generic_wp.unstable_tree->sort (InternalCellInstArrayWithPropertiesBoxConverter (g));
       //  since we use unstable instance trees in non-editable mode, we need to resort the child instances in this case
       sort_child_insts (true);
     }

src/db/db/dbLayout.cc

@@ -1532,6 +1532,8 @@ Layout::rename_cell (cell_index_type id, const char *name)
 bool 
 Layout::topological_sort ()
 {
+  //  NOTE: using cell.instances methods instead of the corresponding cell methods avoids a recursive update call
+
   m_top_cells = 0;
   m_top_down_list.clear ();
 
@@ -1559,7 +1561,7 @@ Layout::topological_sort ()
     //  child cells.
 
     for (const_iterator c = begin (); c != end (); ++c) {
-      if (c->parent_cells () == num_parents [c->cell_index ()]) {
+      if (c->instances ().parent_cells () == num_parents [c->cell_index ()]) {
         m_top_down_list.push_back (c->cell_index ());
         num_parents [c->cell_index ()] = std::numeric_limits<cell_index_type>::max ();
       }
@@ -1568,7 +1570,7 @@ Layout::topological_sort ()
     //  For all these a cells, increment the reported parent instance 
     //  count in all the child cells.
     for (cell_index_vector::const_iterator ii = m_top_down_list.begin () + n_top_down_cells; ii != m_top_down_list.end (); ++ii) {
-      for (cell_type::child_cell_iterator cc = cell (*ii).begin_child_cells (); ! cc.at_end (); ++cc) {
+      for (cell_type::child_cell_iterator cc = cell (*ii).instances ().begin_child_cells (); ! cc.at_end (); ++cc) {
         tl_assert (num_parents [*cc] != std::numeric_limits<cell_index_type>::max ());
         num_parents [*cc] += 1;
       }
@@ -1583,7 +1585,7 @@ Layout::topological_sort ()
   }
 
   //  Determine the number of top cells
-  for (top_down_iterator e = m_top_down_list.begin (); e != m_top_down_list.end () && cell (*e).is_top (); ++e) {
+  for (top_down_iterator e = m_top_down_list.begin (); e != m_top_down_list.end () && cell (*e).instances ().is_top (); ++e) {
     ++m_top_cells;
   }
 
@@ -1815,6 +1817,8 @@ Layout::force_update_no_lock () const
 void 
 Layout::update () const
 {
+  tl::MutexLocker locker (&lock ());
+
   //  NOTE: the assumption is that either one thread is writing or
   //  multiple threads are reading. Hence, we do not need to lock hier_dirty() or bboxes_dirty().
   //  We still do double checking as another thread might do the update as well.
@@ -1822,8 +1826,6 @@ Layout::update () const
     return;
   }
 
-  tl::MutexLocker locker (&lock ());
-
   if (! under_construction ()) {
     force_update_no_lock ();
   }
@@ -1886,13 +1888,14 @@ Layout::do_update ()
         unsigned int layers = 0;
         pr->set (0);
         pr->set_desc (tl::to_string (tr ("Updating bounding boxes")));
-        for (bottom_up_iterator c = begin_bottom_up (); c != end_bottom_up (); ++c) {
+        for (bottom_up_iterator c = m_top_down_list.rbegin (); c != m_top_down_list.rend (); ++c) {
           ++*pr;
           cell_type &cp (cell (*c));
           if (cp.is_shape_bbox_dirty () || dirty_parents.find (*c) != dirty_parents.end ()) {
             if (cp.update_bbox (layers)) {
               //  the bounding box has changed - need to insert parents into "dirty parents" list
-              for (cell_type::parent_cell_iterator p = cp.begin_parent_cells (); p != cp.end_parent_cells (); ++p) {
+              //  NOTE: using "instances" instead of the cell directly avoids a recursive update call
+              for (cell_type::parent_cell_iterator p = cp.instances ().begin_parent_cells (); p != cp.instances ().end_parent_cells (); ++p) {
                 dirty_parents.insert (*p);
               }
             } 
@@ -1907,7 +1910,7 @@ Layout::do_update ()
         tl::SelfTimer timer (tl::verbosity () > layout_base_verbosity + 10, "Sorting shapes");
         pr->set (0);
         pr->set_desc (tl::to_string (tr ("Sorting shapes")));
-        for (bottom_up_iterator c = begin_bottom_up (); c != end_bottom_up (); ++c) {
+        for (bottom_up_iterator c = m_top_down_list.rbegin (); c != m_top_down_list.rend (); ++c) {
           ++*pr;
           cell_type &cp (cell (*c));
           cp.sort_shapes ();
@@ -1922,7 +1925,7 @@ Layout::do_update ()
       size_t layers = 0;
       pr->set (0);
       pr->set_desc (tl::to_string (tr ("Sorting instances")));
-      for (bottom_up_iterator c = begin_bottom_up (); c != end_bottom_up (); ++c) {
+      for (bottom_up_iterator c = m_top_down_list.rbegin (); c != m_top_down_list.rend (); ++c) {
         ++*pr;
         cell_type &cp (cell (*c));
         bool force_sort_inst_tree = dirty_parents.find (*c) != dirty_parents.end ();