[libcxx-commits] [PATCH] D89110: [libcxx][dsl] Reduce number of feature checks

[Alexander Richardson via Phabricator via libcxx-commits]

arichardson created this revision.
arichardson added reviewers: libc++, ldionne.
Herald added subscribers: libcxx-commits, dexonsmith.
Herald added a project: libc++.
Herald added 1 blocking reviewer(s): libc++.
arichardson requested review of this revision.

Even with caching of certain function calls the lit testsuite still takes
a while to start up. It turns out that most cache lookups are caused by
assertions inside the Feature class. This patch introduces a new
tryEnableIn() function. This allows us to only call isSupported() once
rather than currently tow or three times inside newconfig.py configure().

Another way of reducing the number of (potentially) expensive calls would
be to cache the value of the first isSupported() call (see D84055 <https://reviews.llvm.org/D84055>).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89110

Files:
  libcxx/utils/libcxx/test/dsl.py
  libcxx/utils/libcxx/test/newconfig.py


Index: libcxx/utils/libcxx/test/newconfig.py
===================================================================
--- libcxx/utils/libcxx/test/newconfig.py
+++ libcxx/utils/libcxx/test/newconfig.py
@@ -25,9 +25,10 @@
   # Then, apply the automatically-detected features.
   printFeatures = []
   for feature in features:
-    if feature.isSupported(config):
-      feature.enableIn(config)
-      printFeatures.append(feature.getName(config))
+    if feature.tryEnableIn(config):
+      # We can avoid a potentially expensive assertion and use
+      # _getNameUnchecked since we know that the feature is enabled.
+      printFeatures.append(feature._getNameUnchecked(config))
   printFeatures = ["'{}'".format(f) for f in sorted(printFeatures)]
   lit_config.note("Enabling implicitly detected Lit features {}".format(', '.join(printFeatures)))
 
Index: libcxx/utils/libcxx/test/dsl.py
===================================================================
--- libcxx/utils/libcxx/test/dsl.py
+++ libcxx/utils/libcxx/test/dsl.py
@@ -297,6 +297,9 @@
     """
     assert self.isSupported(config), \
       "Trying to get the name of a feature that is not supported in the given configuration"
+    return self._getNameUnchecked(config)
+
+  def _getNameUnchecked(self, config):
     name = self._name(config) if callable(self._name) else self._name
     if not isinstance(name, str):
       raise ValueError("Feature did not resolve to a name that's a string, got {}".format(name))
@@ -316,7 +319,9 @@
     """
     assert self.isSupported(config), \
       "Trying to enable feature {} that is not supported in the given configuration".format(self._name)
+    self._enableInUnchecked(config)
 
+  def _enableInUnchecked(self, config):
     addTo = lambda subs, sub, flag: [(s, x + ' ' + flag) if s == sub else (s, x) for (s, x) in subs]
     if self._compileFlag:
       compileFlag = self._compileFlag(config) if callable(self._compileFlag) else self._compileFlag
@@ -324,8 +329,19 @@
     if self._linkFlag:
       linkFlag = self._linkFlag(config) if callable(self._linkFlag) else self._linkFlag
       config.substitutions = addTo(config.substitutions, '%{link_flags}', linkFlag)
-    config.available_features.add(self.getName(config))
+    config.available_features.add(self._getNameUnchecked(config))
+
+  def tryEnableIn(self, config):
+    """
+    Try to enable a feature in a TestingConfig (if it is supported).
 
+    This is equivalent to `if isSupported(config): enabledIn(config)`, but
+    is slightly faster since it can omit redundant checks.
+    """
+    if self.isSupported(config):
+      self._enableInUnchecked(config)
+      return True
+    return False
 
 def _str_to_bool(s):
   """


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89110.297162.patch
Type: text/x-patch
Size: 2708 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20201009/eaa4a102/attachment-0001.bin>