perf(computes): the create page reads the host's capabilities once

Measured on the create page: 751 ms and 72 libvirt calls, now 123 ms and
13. Its getters parsed the host's capabilities some 60 times, each read
from libvirt. A connection object (one per request) now reads them once,
and the domain capabilities once per emulator, arch, machine and domain
type. The VM page's cached_reads block no longer needs its own copy.

vrtManager/connection.py

@@ -9,6 +9,7 @@ from urllib.parse import quote
 
 import libvirt
 from django.conf import settings
+from django.utils.functional import cached_property
 from libvirt import libvirtError
 from vrtManager import util
 from vrtManager.rwlock import ReadWriteLock
@@ -397,9 +398,19 @@ class wvmConnect(object):
     def is_qemu(self):
         return self.wvm.getURI().startswith("qemu")
 
+    @cached_property
+    def capabilities_xml(self):
+        """The host's capabilities, read once per object (a request): most getters below parse them"""
+        return self.wvm.getCapabilities()
+
+    @cached_property
+    def dom_cap_xmls(self):
+        """The domain capabilities read so far, by emulator, arch, machine and domain type"""
+        return {}
+
     def get_cap_xml(self):
         """Return xml capabilities"""
-        return self.wvm.getCapabilities()
+        return self.capabilities_xml
 
     def get_dom_cap_xml(self, arch, machine):
         """ Return domain capabilities xml"""
@@ -409,7 +420,10 @@ class wvmConnect(object):
         machine_types = self.get_machine_types(arch)
         if not machine or machine not in machine_types:
             machine = "pc" if "pc" in machine_types else machine_types[0]
-        return self.wvm.getDomainCapabilities(emulatorbin, arch, machine, virttype)
+        key = (emulatorbin, arch, machine, virttype)
+        if key not in self.dom_cap_xmls:
+            self.dom_cap_xmls[key] = self.wvm.getDomainCapabilities(*key)
+        return self.dom_cap_xmls[key]
 
     def get_capabilities(self, arch):
         """ Host Capabilities for specified architecture """

vrtManager/instance.py

@@ -491,11 +491,12 @@ class wvmInstance(wvmConnect):
     def cached_reads(self):
         """
         Within the block the domain XML (per flags value), the VM's state,
-        the host's capabilities and info, and each disk's volume are read
-        from libvirt once, for pages that only read: a VM page reads the XML
-        in some 30 getters and each disk in two lists. Code that changes the VM
-        must not run inside: most changes (devices, QoS, memory) do not go
-        through _defineXML, the only call that drops the cache.
+        the host's info and each disk's volume are read from libvirt once
+        (the host's capabilities are read once per object anyway), for pages
+        that only read: a VM page reads the XML in some 30 getters and each
+        disk in two lists. Code that changes the VM must not run inside: most
+        changes (devices, QoS, memory) do not go through _defineXML, the only
+        call that drops the cache.
         """
         if self._read_cache is not None:
             raise RuntimeError("cached_reads blocks do not nest")
@@ -521,13 +522,6 @@ class wvmInstance(wvmConnect):
             self._read_cache.clear()
         return self.wvm.defineXML(xml)
 
-    def get_cap_xml(self):
-        return self._cached("capabilities", super().get_cap_xml)
-
-    def get_dom_cap_xml(self, arch, machine):
-        read = super().get_dom_cap_xml
-        return self._cached(("domain capabilities", arch, machine), lambda: read(arch, machine))
-
     def _host_info(self):
         return self._cached("host info", self.wvm.getInfo)
 

vrtManager/test_capabilities_once.py

@@ -0,0 +1,73 @@
+"""A wvmConnect reads the host's capabilities once, and its domain
+capabilities once per emulator, arch, machine and domain type: the create
+page's getters parse them some 60 times. An object lives for one request."""
+
+import unittest
+
+from django.conf import settings
+
+if not settings.configured:
+    settings.configure(MAC_OUI="52:54:10")
+
+from vrtManager.connection import wvmConnect
+
+CAPABILITIES = """<capabilities><host><cpu><arch>x86_64</arch></cpu></host>
+<guest><os_type>hvm</os_type><arch name='x86_64'><emulator>/usr/bin/qemu-kvm</emulator>
+<machine>pc-q35-9.0</machine><machine canonical='pc-q35-9.0'>q35</machine><machine canonical='pc-i440fx-9.0'>pc</machine>
+<domain type='qemu'/><domain type='kvm'/></arch></guest></capabilities>"""
+
+
+class FakeConn:
+    def __init__(self):
+        self.reads = []
+
+    def getCapabilities(self):
+        self.reads.append("capabilities")
+        return CAPABILITIES
+
+    def getDomainCapabilities(self, emulator, arch, machine, virttype):
+        self.reads.append((emulator, arch, machine, virttype))
+        return "<domainCapabilities/>"
+
+
+def connection():
+    conn = wvmConnect.__new__(wvmConnect)  # no libvirt connection: only these calls
+    conn.wvm = FakeConn()
+    return conn
+
+
+class CapabilitiesOnceTestCase(unittest.TestCase):
+    def test_the_capabilities_are_read_once_for_all_getters(self):
+        conn = connection()
+        for _ in range(2):
+            conn.get_emulator("x86_64")
+            conn.get_machine_types("x86_64")
+            conn.get_hypervisors_domain_types()
+            conn.get_cap_xml()
+        self.assertEqual(conn.wvm.reads, ["capabilities"])
+
+    def test_domain_capabilities_are_read_once_per_machine(self):
+        conn = connection()
+        for _ in range(2):
+            conn.get_dom_cap_xml("x86_64", "q35")
+            conn.get_dom_cap_xml("x86_64", "pc")
+            conn.get_dom_cap_xml("x86_64", "unknown")  # falls back to pc
+        self.assertEqual(
+            conn.wvm.reads,
+            ["capabilities", ("/usr/bin/qemu-kvm", "x86_64", "q35", "kvm"), ("/usr/bin/qemu-kvm", "x86_64", "pc", "kvm")],
+        )
+
+    def test_two_objects_do_not_share_their_reads(self):
+        a, b = connection(), connection()
+        a.get_cap_xml()
+        b.get_cap_xml()
+        self.assertEqual((a.wvm.reads, b.wvm.reads), (["capabilities"], ["capabilities"]))
+
+    def test_a_failed_read_is_not_kept(self):
+        conn = connection()
+        conn.wvm.getCapabilities = lambda: (_ for _ in ()).throw(RuntimeError("libvirt error"))
+        with self.assertRaises(RuntimeError):
+            conn.get_cap_xml()
+        conn.wvm = FakeConn()
+        conn.get_cap_xml()
+        self.assertEqual(conn.wvm.reads, ["capabilities"])

vrtManager/test_read_cache.py

@@ -1,10 +1,10 @@
 """wvmInstance.cached_reads(): inside the block the domain XML (per flags),
-the VM's state, the host capabilities and info, and each disk's volume are
-read from libvirt once; outside it every read goes to libvirt, as code that
-changes the VM needs."""
+the VM's state, the host info and each disk's volume are read from libvirt
+once; outside it every read goes to libvirt, as code that changes the VM
+needs. The host capabilities are read once per object."""
 
 import unittest
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock
 
 from django.conf import settings
 
@@ -12,7 +12,6 @@ if not settings.configured:
     settings.configure(MAC_OUI="52:54:10")
 
 from libvirt import libvirtError
-from vrtManager.connection import wvmConnect
 from vrtManager.instance import wvmInstance
 
 
@@ -61,8 +60,8 @@ class CachedReadsTestCase(unittest.TestCase):
         p = proxy()
         p._XMLDesc(0)
         p._XMLDesc(0)
-        p.get_cap_xml()
-        p.get_cap_xml()
+        p._host_info()
+        p._host_info()
         self.assertEqual((p.instance.reads, p.wvm.caps), ([0, 0], 2))
 
     def test_inside_the_block_each_flags_value_is_read_once(self):
@@ -70,8 +69,8 @@ class CachedReadsTestCase(unittest.TestCase):
         with p.cached_reads():
             first = [p._XMLDesc(0), p._XMLDesc(1), p._XMLDesc(3)]
             again = [p._XMLDesc(0), p._XMLDesc(1), p._XMLDesc(3)]
-            p.get_cap_xml()
-            p.get_cap_xml()
+            p._host_info()
+            p._host_info()
         self.assertEqual(first, again)
         self.assertEqual(p.instance.reads, [0, 1, 3])
         self.assertEqual(p.wvm.caps, 1)
@@ -135,17 +134,6 @@ class CachedReadsTestCase(unittest.TestCase):
         p.get_status()
         self.assertEqual(p.instance.reads, ["info", "info"])
 
-    def test_domain_capabilities_are_read_once_per_arch_and_machine(self):
-        p = proxy()
-        with patch.object(wvmConnect, "get_dom_cap_xml", return_value="<domainCapabilities/>") as read:
-            with p.cached_reads():
-                for _ in range(2):
-                    p.get_dom_cap_xml("x86_64", "q35")
-                    p.get_dom_cap_xml("x86_64", "pc")
-            p.get_dom_cap_xml("x86_64", "q35")
-        self.assertEqual([c.args for c in read.call_args_list], [("x86_64", "q35"), ("x86_64", "pc"), ("x86_64", "q35")])
-
-
 def volume_proxy(paths):
     """A proxy whose host has volumes at paths (pool "pool", size 10, used 4)."""
     p = proxy()