VM.all is not cached, and VM.find makes additional commands

[KieranP]

VM.all doesn't appear cached. So if I do a .all for the menu, and later a .find in the same request, the find make an additional command, rather than reusing the output from .all

This slows things a far bit, esp when making ~5-6 calls in a single request.

[AlekSi]

Personally I don't think command invocations should be cached at all. What if you create new machine from VirtualBox GUI?

[KieranP]

Then it should clear all cached value so it populates on the next call.

[mitchellh]

VM objects do take quite awhile to build up. It may make sense to add the reload parameter to VM.all, similar to how VM#state and ExtraData.global work at the moment. What do you think of that?

[KieranP]

Mitchell, sounds great. I'm also noticing it makes additional commands to build relationships. It would be nice to turn these off, since In the case of my app, the menu, with 20 VMs, takes ages to load because of the relationship commands (there is 2 relationship commands per vm? That makes around 41 system calls!!). The truth is, I only need the name, uuid, and state, so :without_relationships => true option would be very helpful.

[KieranP]

So it looks to me like there is two optimizations here:

Firstly, "list vms" returns a short list of names and uuid, which for each one, showvminfo is then called.

Why not parse "list --long vms" and remove calls to --machinereadable altogether? You make one command, get all the info you need, and save dozens of command calls for larger data sets.

Secondly, the relationships for things like NIC go ahead and call additional commands.

So basically, if you have all 8 network adapters running, on 25 VMs, to just list their name, ostype, state, and uuid, you're looking at anywhere between 200 and 250 commands. Ideally, should be able to get away with only 1 (to "list --long vms").

[mitchellh]

KieranP,

I see what you're saying. But parsing the "non-machine-readable" version of the command introduces much more complexity than the machine readable version (as is to be expected).

I can see you made a web interface to vbox which is awesome, and is obviously a use case where performance is pretty important.

But I'm not convinced that the optimization of VM.all to parse the nonmachinereadable output of VBoxManage, and to have that data propagate across relationships is worth it at this point in development.

I'm not certain but I think most use cases wouldn't be calling VM.all too often, and even if they did, it would be once and that would be that. They would cache the value or something to avoid calling it again.

I may still add the reload parameter to it and do a simple cache of the data, but I don't think its worth the time and effort at the moment to do a major optimization overhaul, when there are still features of VirtualBox not covered by the library.

As always, you're welcome to give this a shot yourself :) but I think I'm gonna push this back.

Mitchell

[KieranP]

Since the change to parse the XML, there has been a massive speed boost! Great job. Sadly, the VM state and HD size/accessible will always need system calls, as the VBox devs won't add that data to the XML files :-( Never-the-less, some awesome work on this. It also looks like VM.all and HD.all are now cached, and that .find calls select from the .all results. Nice. Closing this ticket now that this is resolved.

[mitchellh]

Agreed. This has been taken care of.

Thanks. There are a few issues left that concern the new XML parsing but those can be solved with other tickets :)