Speed up device comparisons in netlist compare

src/db/db/dbDevice.cc

@@ -163,11 +163,9 @@ void Device::connect_terminal (size_t terminal_id, Net *net)
   }
 }
 
-double Device::parameter_value (size_t param_id) const
+double Device::parameter_default_value (size_t param_id) const
 {
-  if (m_parameters.size () > param_id) {
-    return m_parameters [param_id];
-  } else if (mp_device_class) {
+  if (mp_device_class) {
     const db::DeviceParameterDefinition *pd = mp_device_class->parameter_definition (param_id);
     if (pd) {
       return pd->default_value ();

src/db/db/dbDevice.h

@@ -282,8 +282,13 @@ class DB_PUBLIC Device
 
   /**
    *  @brief Gets the value for the parameter with the given ID
+   *  NOTE: this is a hot path in the netlist compare - the fast path (parameter
+   *  present in the parameter vector) is inline, the fallback is not.
    */
-  double parameter_value (size_t param_id) const;
+  double parameter_value (size_t param_id) const
+  {
+    return m_parameters.size () > param_id ? m_parameters [param_id] : parameter_default_value (param_id);
+  }
 
   /**
    *  @brief Sets the value for the parameter with the given ID
@@ -397,6 +402,12 @@ class DB_PUBLIC Device
   friend class Circuit;
   friend class Net;
 
+  /**
+   *  @brief Gets the default value of the parameter with the given ID
+   *  Returns 0 if there is no such parameter.
+   */
+  double parameter_default_value (size_t param_id) const;
+
   DeviceClass *mp_device_class;
   DeviceAbstract *mp_device_abstract;
   std::string m_name;

src/db/db/dbDeviceClass.cc

@@ -25,6 +25,8 @@
 #include "dbNetlist.h"
 #include "tlClassRegistry.h"
 
+#include <algorithm>
+
 namespace db
 {
 
@@ -309,29 +311,24 @@ size_t DeviceClass::terminal_id_for_name (const std::string &name) const
 //  The default compare delegate
 static EqualDeviceParameters default_compare;
 
-bool DeviceClass::less (const db::Device &a, const db::Device &b)
+static const db::DeviceParameterCompareDelegate *
+compare_delegate (const db::Device &a, const db::Device &b)
 {
   tl_assert (a.device_class () != 0);
   tl_assert (b.device_class () != 0);
 
   const db::DeviceParameterCompareDelegate *pcd = primary_device_class (a, b)->parameter_compare_delegate ();
-  if (! pcd) {
-    pcd = &default_compare;
-  }
+  return pcd ? pcd : &default_compare;
+}
 
-  return pcd->less (a, b);
+bool DeviceClass::less (const db::Device &a, const db::Device &b)
+{
+  return compare_delegate (a, b)->less (a, b);
 }
 
 bool DeviceClass::equal (const db::Device &a, const db::Device &b)
 {
-  tl_assert (a.device_class () != 0);
-  tl_assert (b.device_class () != 0);
-
-  const db::DeviceParameterCompareDelegate *pcd = primary_device_class (a, b)->parameter_compare_delegate ();
-  if (! pcd) {
-    pcd = &default_compare;
-  }
-
+  const auto *pcd = compare_delegate (a, b);
   return ! pcd->less (a, b) && ! pcd->less (b, a);
 }
 

src/db/db/dbNetlistCompareGraph.cc

@@ -113,10 +113,15 @@ Transition::operator< (const Transition &other) const
     }
 
     if (device () != 0) {
+
+      //  a single three-way comparison replaces the "equals" plus "less"
+      //  calls and skips the parameter comparison entirely for identical devices
       DeviceCompare dc;
-      if (! dc.equals (std::make_pair (device (), cat ()), std::make_pair (other.device (), other.cat ()))) {
-        return dc (std::make_pair (device (), cat ()), std::make_pair (other.device (), other.cat ()));
+      int cmp = dc.compare (std::make_pair (device (), cat ()), std::make_pair (other.device (), other.cat ()));
+      if (cmp != 0) {
+        return cmp < 0;
       }
+
     }
 
     if (m_id1 != other.m_id1) {
@@ -134,6 +139,12 @@ Transition::operator== (const Transition &other) const
     return false;
   }
 
+  //  the ID comparison is a pure conjunction with the other predicates, so
+  //  do the cheap field first
+  if (m_id1 != other.m_id1) {
+    return false;
+  }
+
   if (is_for_subcircuit ()) {
 
     if ((subcircuit () != 0) != (other.subcircuit () != 0)) {
@@ -147,7 +158,7 @@ Transition::operator== (const Transition &other) const
       }
     }
 
-    return (m_id1 == other.m_id1);
+    return true;
 
   } else {
 
@@ -156,13 +167,16 @@ Transition::operator== (const Transition &other) const
     }
 
     if (device () != 0) {
+
+      //  a single three-way comparison replaces the "equals" call
       DeviceCompare dc;
-      if (! dc.equals (std::make_pair (device (), cat ()), std::make_pair (other.device (), other.cat ()))) {
+      if (dc.compare (std::make_pair (device (), cat ()), std::make_pair (other.device (), other.cat ())) != 0) {
         return false;
       }
+
     }
 
-    return (m_id1 == other.m_id1 && m_id2 == other.m_id2);
+    return m_id2 == other.m_id2;
 
   }
 }

src/db/db/dbNetlistCompareUtils.cc

@@ -197,9 +197,32 @@ DeviceCompare::equals (const std::pair<const db::Device *, size_t> &d1, const st
   if (d1.second != d2.second) {
     return false;
   }
+  //  identical devices are equal under any tolerance scheme
+  if (d1.first == d2.first) {
+    return true;
+  }
   return db::DeviceClass::equal (*d1.first, *d2.first);
 }
 
+int
+DeviceCompare::compare (const std::pair<const db::Device *, size_t> &d1, const std::pair<const db::Device *, size_t> &d2) const
+{
+  if (d1.second != d2.second) {
+    return d1.second < d2.second ? -1 : 1;
+  }
+  //  identical devices are equal under any tolerance scheme
+  if (d1.first == d2.first) {
+    return 0;
+  }
+  if (db::DeviceClass::less (*d1.first, *d2.first)) {
+    return -1;
+  } else if (db::DeviceClass::less (*d2.first, *d1.first)) {
+    return 1;
+  } else {
+    return 0;
+  }
+}
+
 // --------------------------------------------------------------------------------------------------------------------
 //  SubCircuitCompare implementation
 

src/db/db/dbNetlistCompareUtils.h

@@ -132,6 +132,15 @@ struct DB_PUBLIC DeviceCompare
 {
   bool operator() (const std::pair<const db::Device *, size_t> &d1, const std::pair<const db::Device *, size_t> &d2) const;
   bool equals (const std::pair<const db::Device *, size_t> &d1, const std::pair<const db::Device *, size_t> &d2) const;
+
+  /**
+   *  @brief Three-way comparison of the devices
+   *
+   *  This combines "equals" and the "less" operation into a single call:
+   *  it returns -1 if d1 < d2, 1 if d1 > d2 and 0 if both are equal
+   *  under the same criterion "equals" applies.
+   */
+  int compare (const std::pair<const db::Device *, size_t> &d1, const std::pair<const db::Device *, size_t> &d2) const;
 };
 
 // --------------------------------------------------------------------------------------------------------------------

src/db/unit_tests/dbNetlistCompareTests.cc

@@ -438,6 +438,131 @@ TEST(0_EqualDeviceParameters)
   EXPECT_EQ (dc.less (d2, d1), false);
 }
 
+//  a compare delegate with a fixed "less" result, independent of the parameters
+//  (like GenericDeviceParameterCompare, it overrides EqualDeviceParameters::less)
+class FixedLessCompare
+  : public db::EqualDeviceParameters
+{
+public:
+  FixedLessCompare (const db::Device *first, bool less_first_second, bool less_second_first)
+    : m_first (first), m_less_first_second (less_first_second), m_less_second_first (less_second_first)
+  { }
+
+  virtual bool less (const db::Device &a, const db::Device &b) const
+  {
+    return &a == m_first ? m_less_first_second : m_less_second_first;
+  }
+
+private:
+  const db::Device *m_first;
+  bool m_less_first_second, m_less_second_first;
+};
+
+//  asserts that compare, less and equal agree on the delegate's "less" answers
+static void expect_compare (tl::TestBase *_this, const db::Device &a, const db::Device &b, bool less_ab, bool less_ba)
+{
+  //  same category on both sides, so the functor takes the pure device decision
+  const size_t cat = 0;
+  db::DeviceCompare dc;
+  EXPECT_EQ (dc.compare (std::make_pair (&a, cat), std::make_pair (&b, cat)), less_ab ? -1 : (less_ba ? 1 : 0));
+  EXPECT_EQ (dc.compare (std::make_pair (&b, cat), std::make_pair (&a, cat)), less_ba ? -1 : (less_ab ? 1 : 0));
+  EXPECT_EQ (db::DeviceClass::less (a, b), less_ab);
+  EXPECT_EQ (db::DeviceClass::less (b, a), less_ba);
+  EXPECT_EQ (db::DeviceClass::equal (a, b), ! less_ab && ! less_ba);
+  EXPECT_EQ (db::DeviceClass::equal (b, a), ! less_ab && ! less_ba);
+}
+
+TEST(0_DeviceClassCompare)
+{
+  db::DeviceClassMOS3Transistor dc;
+
+  db::Device d1 (&dc);
+  db::Device d2 (&dc);
+
+  //  default delegate: primary parameters only, default tolerance
+
+  d1.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 40.0);
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 40.0);
+  d1.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_W, 20.0);
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_W, 20.0);
+
+  //  non-primary parameters are not compared, and neither are parameters beyond the parameter definitions
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_AD, 1.0);
+  d1.set_parameter_value (100, 5.0);
+
+  expect_compare (_this, d1, d2, false, false);
+
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_W, 21.0);
+  expect_compare (_this, d1, d2, true, false);
+
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_W, 20.0);
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 41.0);
+  expect_compare (_this, d1, d2, true, false);
+
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 39.0);
+  expect_compare (_this, d1, d2, false, true);
+
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 40.0);
+
+  //  explicit delegate with a tolerance and an ignored parameter
+
+  db::EqualDeviceParameters *eqp = new db::EqualDeviceParameters ();
+  *eqp += db::EqualDeviceParameters (db::DeviceClassMOS3Transistor::param_id_L, 0.5, 0.0);
+  *eqp += db::EqualDeviceParameters (db::DeviceClassMOS3Transistor::param_id_W, true);  //  ignore W
+  dc.set_parameter_compare_delegate (eqp);
+
+  //  W is ignored
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_W, 30.0);
+  expect_compare (_this, d1, d2, false, false);
+
+  //  L is inside the absolute tolerance
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 40.4);
+  expect_compare (_this, d1, d2, false, false);
+
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 40.6);
+  expect_compare (_this, d1, d2, true, false);
+
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 39.4);
+  expect_compare (_this, d1, d2, false, true);
+
+  d2.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_L, 40.0);
+
+  //  a non-primary parameter is compared when listed in the compare set
+  *eqp += db::EqualDeviceParameters (db::DeviceClassMOS3Transistor::param_id_AD, 0.0, 0.0);
+
+  expect_compare (_this, d1, d2, true, false);
+
+  d1.set_parameter_value (db::DeviceClassMOS3Transistor::param_id_AD, 1.0);
+  expect_compare (_this, d1, d2, false, false);
+
+  //  a parameter beyond the parameter definitions can be put into the compare set
+  //  (d1 carries 5.0 there, d2 the default value 0.0)
+  *eqp += db::EqualDeviceParameters (100, 0.25, 0.0);
+
+  expect_compare (_this, d1, d2, false, true);
+
+  d2.set_parameter_value (100, 5.1);
+  expect_compare (_this, d1, d2, false, false);
+
+  d2.set_parameter_value (100, 5.4);
+  expect_compare (_this, d1, d2, true, false);
+
+  //  a delegate subclass overriding "less" (like GenericDeviceParameterCompare does)
+
+  dc.set_parameter_compare_delegate (new FixedLessCompare (&d1, false, false));
+  expect_compare (_this, d1, d2, false, false);
+
+  dc.set_parameter_compare_delegate (new FixedLessCompare (&d1, true, false));
+  expect_compare (_this, d1, d2, true, false);
+
+  dc.set_parameter_compare_delegate (new FixedLessCompare (&d1, false, true));
+  expect_compare (_this, d1, d2, false, true);
+
+  //  "less" in both directions: compare takes the first one, so equal is false
+  dc.set_parameter_compare_delegate (new FixedLessCompare (&d1, true, true));
+  expect_compare (_this, d1, d2, true, true);
+}
+
 TEST(0_NetNameEquivalence)
 {
   db::Netlist a, b;