Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to speed up qvm-ls #165

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ ADMIN_API_METHODS_SIMPLE = \
admin.pool.volume.Revert \
admin.pool.volume.Snapshot \
admin.property.Get \
admin.property.GetAll \
admin.property.GetDefault \
admin.property.Help \
admin.property.HelpRst \
Expand All @@ -45,6 +46,7 @@ ADMIN_API_METHODS_SIMPLE = \
admin.vm.CreateInPool.StandaloneVM \
admin.vm.CreateInPool.TemplateVM \
admin.vm.CreateDisposable \
admin.vm.GetAllData \
admin.vm.Kill \
admin.vm.List \
admin.vm.Pause \
Expand Down Expand Up @@ -79,6 +81,7 @@ ADMIN_API_METHODS_SIMPLE = \
admin.vm.firewall.SetPolicy \
admin.vm.firewall.Reload \
admin.vm.property.Get \
admin.vm.property.GetAll \
admin.vm.property.GetDefault \
admin.vm.property.Help \
admin.vm.property.HelpRst \
Expand Down
71 changes: 55 additions & 16 deletions qubes/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ def __init__(self, name, setter=None, saver=None, type=None,
lambda self, prop, value: str(value))
self.type = type
self._default = default
self._default_function = None
if isinstance(default, collections.Callable):
self._default_function = default

self._write_once = write_once
self.order = order
self.load_stage = load_stage
Expand All @@ -223,12 +227,15 @@ def __get__(self, instance, owner):
except AttributeError:
return self.get_default(instance)

def has_default(self):
return self._default is not self._NO_DEFAULT

def get_default(self, instance):
if self._default is self._NO_DEFAULT:
raise AttributeError(
'property {!r} have no default'.format(self.__name__))
elif isinstance(self._default, collections.Callable):
return self._default(instance)
elif self._default_function:
return self._default_function(instance)
else:
return self._default

Expand Down Expand Up @@ -492,7 +499,7 @@ def __init__(self, xml, **kwargs):

propvalues = {}

all_names = set(prop.__name__ for prop in self.property_list())
all_names = self.property_dict()
for key in list(kwargs):
if not key in all_names:
continue
Expand All @@ -505,15 +512,46 @@ def __init__(self, xml, **kwargs):

if self.xml is not None:
# check if properties are appropriate
all_names = set(prop.__name__ for prop in self.property_list())

for node in self.xml.xpath('./properties/property'):
name = node.get('name')
if name not in all_names:
raise TypeError(
'property {!r} not applicable to {!r}'.format(
name, self.__class__.__name__))

# pylint: disable=too-many-nested-blocks
@classmethod
def property_dict(cls, load_stage=None):
'''List all properties attached to this VM's class

:param load_stage: Filter by load stage
:type load_stage: :py:func:`int` or :py:obj:`None`
'''

# use cls.__dict__ since we must not look at parent classes
if "_property_dict" not in cls.__dict__:
cls._property_dict = {}
memo = cls._property_dict

if load_stage not in memo:
props = dict()
if load_stage is None:
for class_ in cls.__mro__:
for name in class_.__dict__:
# don't overwrite props with those from base classes
if name not in props:
prop = class_.__dict__[name]
if isinstance(prop, property):
assert name == prop.__name__
props[name] = prop
else:
for prop in cls.property_dict().values():
if prop.load_stage == load_stage:
props[prop.__name__] = prop
memo[load_stage] = props

return memo[load_stage]

@classmethod
def property_list(cls, load_stage=None):
'''List all properties attached to this VM's class
Expand All @@ -522,14 +560,15 @@ def property_list(cls, load_stage=None):
:type load_stage: :py:func:`int` or :py:obj:`None`
'''

props = set()
for class_ in cls.__mro__:
props.update(prop for prop in class_.__dict__.values()
if isinstance(prop, property))
if load_stage is not None:
props = set(prop for prop in props
if prop.load_stage == load_stage)
return sorted(props)
# use cls.__dict__ since we must not look at parent classes
if "_property_list" not in cls.__dict__:
cls._property_list = {}
memo = cls._property_list

if load_stage not in memo:
memo[load_stage] = sorted(cls.property_dict(load_stage).values())

return memo[load_stage]

def _property_init(self, prop, value):
'''Initialise property to a given value, without side effects.
Expand Down Expand Up @@ -584,9 +623,9 @@ def property_get_def(cls, prop):
if isinstance(prop, qubes.property):
return prop

for p in cls.property_list():
if p.__name__ == prop:
return p
props = cls.property_dict()
if prop in props:
return props[prop]

raise AttributeError('No property {!r} found in {!r}'.format(
prop, cls))
Expand Down
144 changes: 128 additions & 16 deletions qubes/api/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,18 @@ def on_domain_delete(self, subject, event, vm):
vm.remove_handler('*', self.vm_handler)


def escape(unescaped_string):
'''Escape a string for the Admin API'''
result = unescaped_string
# TODO: can we do this faster?
result = result.replace("\\", "\\\\") # this must be the first one

result = result.replace("\r", "\\r")
result = result.replace("\n", "\\n")
result = result.replace("\t", "\\t")
result = result.replace("\0", "\\0")
return result

class QubesAdminAPI(qubes.api.AbstractQubesAPI):
'''Implementation of Qubes Management API calls

Expand All @@ -107,6 +119,15 @@ def vmclass_list(self):
return ''.join('{}\n'.format(ep.name)
for ep in entrypoints)

# pylint: disable=no-self-use
def _vm_line_strs(self, strs, vm):
strs.append(vm.name)
strs.append(" class=")
strs.append(vm.__class__.__name__)
strs.append(" state=")
strs.append(vm.get_power_state())
strs.append("\n")

@qubes.api.method('admin.vm.List', no_payload=True,
scope='global', read=True)
@asyncio.coroutine
Expand All @@ -119,11 +140,37 @@ def vm_list(self):
else:
domains = self.fire_event_for_filter([self.dest])

return ''.join('{} class={} state={}\n'.format(
vm.name,
vm.__class__.__name__,
vm.get_power_state())
for vm in sorted(domains))
strs = []
for vm in sorted(domains):
self._vm_line_strs(strs, vm)
return ''.join(strs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is str.append really faster than str.format?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to avoid doing any kind of string processing until the final join call, which should be the fastest since it should just allocate a single string in join and copy them all in, avoiding any intermediate allocations, partial concatenations, format string parsing, etc.

It's slightly uglier, but not that much, it looks like writing to an output stream (which would be even faster, but not really worth the effort).


@qubes.api.method('admin.vm.GetAllData', no_payload=True,
scope='global', read=True)
@asyncio.coroutine
def vm_get_all_data(self):
'''List all VMs and return the value of all their properties'''
assert not self.arg

if self.dest.name == 'dom0':
domains = self.fire_event_for_filter(self.app.domains)
else:
domains = self.fire_event_for_filter([self.dest])

strs = []
orig_dest = self.dest
orig_method = self.method
try:
for vm in sorted(domains):
self.dest = vm
self.method = "admin.vm.property.GetAll"
self._vm_line_strs(strs, vm)
self._property_get_all_strs(strs, vm, "\tP\t")
finally:
self.dest = orig_dest
self.method = orig_method

return ''.join(strs)

@qubes.api.method('admin.vm.property.List', no_payload=True,
scope='local', read=True)
Expand Down Expand Up @@ -154,6 +201,13 @@ def vm_property_get(self):
'''Get a value of one property'''
return self._property_get(self.dest)

@qubes.api.method('admin.vm.property.GetAll', no_payload=True,
scope='local', read=True)
@asyncio.coroutine
def vm_property_get_all(self):
'''Get the value of all properties'''
return self._property_get_all(self.dest)

@qubes.api.method('admin.property.Get', no_payload=True,
scope='global', read=True)
@asyncio.coroutine
Expand All @@ -162,24 +216,39 @@ def property_get(self):
assert self.dest.name == 'dom0'
return self._property_get(self.app)

def _property_get(self, dest):
if self.arg not in dest.property_list():
raise qubes.exc.QubesNoSuchPropertyError(dest, self.arg)

self.fire_event_for_permission()
@qubes.api.method('admin.property.GetAll', no_payload=True,
scope='global', read=True)
@asyncio.coroutine
def property_get_all(self):
'''Get a value of all global properties'''
assert self.dest.name == 'dom0'
return self._property_get_all(self.app)

property_def = dest.property_get_def(self.arg)
# pylint: disable=no-self-use
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@staticmethod

def _property_type(self, property_def):
# explicit list to be sure that it matches protocol spec
if isinstance(property_def, qubes.vm.VMProperty):
property_type = 'vm'
elif property_def.type is int:
property_def_type = property_def.type
if property_def_type is str:
property_type = 'str'
elif property_def_type is int:
property_type = 'int'
elif property_def.type is bool:
elif property_def_type is bool:
property_type = 'bool'
elif self.arg == 'label':
elif property_def.__name__ == 'label':
property_type = 'label'
elif isinstance(property_def, qubes.vm.VMProperty):
property_type = 'vm'
else:
property_type = 'str'
return property_type

def _property_get(self, dest):
if self.arg not in dest.property_list():
raise qubes.exc.QubesNoSuchPropertyError(dest, self.arg)

self.fire_event_for_permission()
property_def = dest.property_get_def(self.arg)
property_type = self._property_type(property_def)

try:
value = getattr(dest, self.arg)
Expand All @@ -191,6 +260,49 @@ def _property_get(self, dest):
property_type,
str(value) if value is not None else '')


# this is a performance critical function for qvm-ls
def _property_get_all_line_strs(self, strs, dest, property_def):
property_type = self._property_type(property_def)
property_attr = property_def._attr_name # pylint: disable=protected-access
dest_dict = dest.__dict__
property_is_default = property_attr not in dest_dict

if not property_is_default:
value = dest_dict[property_attr]
elif property_def.has_default():
value = property_def.get_default(dest)
else:
value = None

if value is None:
escaped_value = ""
elif property_type == "str":
escaped_value = escape(str(value))
else:
escaped_value = str(value)

strs.append(property_def.__name__)
strs.append("\tD\t" if property_is_default else "\t-\t")
strs.append(property_type)
strs.append("\t")
strs.append(escaped_value)
strs.append("\n")

def _property_get_all_strs(self, strs, dest, prefix=None):
assert not self.arg

properties = self.fire_event_for_filter(dest.property_list())
for prop in properties:
if prefix:
strs.append(prefix)
self._property_get_all_line_strs(strs, dest, prop)

def _property_get_all(self, dest):
strs = []
self._property_get_all_strs(strs, dest)
return "".join(strs)

@qubes.api.method('admin.vm.property.GetDefault', no_payload=True,
scope='local', read=True)
@asyncio.coroutine
Expand Down
Loading