Skip to content
This repository has been archived by the owner on Sep 4, 2020. It is now read-only.

Performance of gathering inventory objects #114

Closed
brianbunke opened this issue Mar 30, 2017 · 6 comments
Closed

Performance of gathering inventory objects #114

brianbunke opened this issue Mar 30, 2017 · 6 comments

Comments

@brianbunke
Copy link
Contributor

Invoke-Vester could be a lot faster. You all are really polite for going this entire month of March trying out New Vester and not commenting on the performance. 😄

Currently, when each new test is ready to run, it checks its scope and then pulls the objects it will apply to. So, if a VM test is about to run, part that test's prep is to Get-VM and then ForEach the test against all applicable VMs.

This sucks when running the full test suite. For many VM tests in a row, you have to Get-VM at the beginning of each one.

There are two options:

  1. Current implementation
  2. Get- all inventory objects once, store them in variables, and pass them into each test

I did 1 initially because 2 means even if you're only running Host tests, you still have to Get-VM, Get-Cluster, etc. I'm over caring about that at this point. (and I may be able to weasel around that anyway, haha)

@brianbunke brianbunke self-assigned this Mar 30, 2017
@midacts
Copy link
Contributor

midacts commented Mar 31, 2017

I like the idea of option two as well.

I did a little digging to understand a little of how things work behind the scenes.
What about in VesterTemplate.Tests.ps1 if you grabbed all the scopes that are going to be used at the very start (Line 25 maybe - before the foreach($Test in $TestFiles)):
$Scopes = Split-Path (Split-Path $TestFiles -parent) -leaf | Select -Unique

And then move lines 65-76 below $Scopes on line 25.
Instead of checking the switch once, we could do a foreach loop and get only the scopes we care about.

Something like this maybe?
https://gist.github.com/Midacts/4fe777f81f823523f118a9c4437a618f

@brianbunke
Copy link
Contributor Author

Sounds pretty good. 😃

@brianbunke brianbunke removed their assignment Apr 6, 2017
@jpsider
Copy link
Contributor

jpsider commented Apr 8, 2017

I did some work and testing around the 'Get' performance, I ended up making too many changes for a single PR, all of those changes are here: https://github.com/jpsider/Vester/blob/Improve-Get-Performance/Vester/Private/Template/VesterTemplate.Tests.ps1

I will be working on the changes that I found to be directly related to performance, and will submit a PR for that. I will also open up some new issues that resulted from the research and testing.

Testing numbers :
Before (Local lab) - 5 vms
Tests completed in 8.12s
Passed: 52 Failed: 1 Skipped: 0 Pending: 0 Inconclusive: 0

After (Local lab) - 5 vms
Tests completed in 5.5s
Passed: 52 Failed: 1 Skipped: 0 Pending: 0 Inconclusive: 0

before - 240 vms
Tests completed in 35.29s
Passed: 805 Failed: 252 Skipped: 0 Pending: 0 Inconclusive: 0

after - 240 vms
Tests completed in 32.97s
Passed: 805 Failed: 252 Skipped: 0 Pending: 0 Inconclusive: 0

Before (VPN) - 6vms
Tests completed in 26.87s
Passed: 67 Failed: 10 Skipped: 0 Pending: 0 Inconclusive: 0

After (VPN) - 6 vms
Tests completed in 13.79s
Passed: 67 Failed: 10 Skipped: 0 Pending: 0 Inconclusive: 0

Before (VPN) - 200 vms
Tests completed in 72.05s
Passed: 467 Failed: 410 Skipped: 0 Pending: 0 Inconclusive: 0

After (VPN) - 200 vms
Tests completed in 60.21s
Passed: 467 Failed: 410 Skipped: 0 Pending: 0 Inconclusive: 0

@midacts
Copy link
Contributor

midacts commented Apr 9, 2017

https://gist.github.com/Midacts/91406149a003cb61e8137ec0ff140232

I've discussed these changes in slack, but to reiterate here:

TL;DR
We loop through each scope and make an object with the:
-scope name
-objects from the scope: Get-VM or Get-Cluster etc
-Test files for that scope

We then add that object to the main object that will hold the object for all the scopes -> $Final

We remove scopes we don't need.

Then we loop through every scope

Loop through all the inventory objects for that scope

Run each test in that scope against that inventory object and then do the same thing for the
remaining inventory objects

This makes it so the 'gets': Get-VM, Get-Cluster, etc are only run one time AND we are only running 'gets' and tests for scopes that are referenced (either everything, or limited to the scopes using the -Test parameter).

Breakdown of changes

  • Line 26
    these changes first pull in all the scopes (vm, host, cluster, etc)
    it is able to pull in all the relative scopes by splitting the $testfiles parameter

  • Line 27 and 28
    This is to create these arrays for later

  • Line 29
    Might as well get the datacenter at the very start (and outside of any foreach loop) since every test is dependent on $Datacenter

  • Line 30-52
    Loops through each of the scopes (vm, host, cluster, etc)
    it runs the get for that scope -> if the scope is VM, it runs Line 42 and runs Get-VM.

After it gets the scope (VM, etc) and the inventory list (the Get-VM output, etc) it throws all that data into a pscustomobject named $ScopeObjs (Line 46).
It also puts all the tests files related to that object in $ScopeObj:
Scope InventoryList TestFiles
----- ------------- ---------
VM {VM1, VM2...} {CDDrive-Host.Vester.ps1,CDDrive-ISO.Vester...}

  • Line 51
    It adds the output of each scope ($ScopeObj) into one object named $Final

  • Line 55
    Condenses $Final down to only the scopes that have inventory objects to test and test files.
    So if a scope does not have any test files in it or its inventory is empty (like the network scope in my scenario) it will remove it from $Final, which will save time from having to loop through a blank scope.

  • Line 58
    I did scope because our tests currently run by scope.

  • Line 61 and 62
    $Inventory -> pulls only the objects relevant for this scope and stores it in $Inventory
    $Tests -> pulls only the test files for this scope and stores it in $Tests

  • Line 64 - 73
    This is a carry over.
    I think we can remove it based on Line 55
    Also, I had to reverse the -notmatch sequence. For brevity sake, if you test -notmatch you will see why i made that change

  • Line 75-87
    This is a carry over
    I think it needs to stay. I dont know of any other place to put it, but it is definitely a needed check for the Network scope

  • Line 89-93
    This is a carry over.
    I think we can remove this line based on Line 55

  • Line 96
    Now that we are in a scope (VM, cluster, etc) we get the inventory objects of that scope (the outputs from the Get-VM, Get-Cluster, etc) and loop through them one at a time.

  • Line 99
    For each Inventory object, we run every test in that scope against that object

Everything after line 99 is carry over code

@jpsider
Copy link
Contributor

jpsider commented Apr 9, 2017

I have two small edits

  • Move the check for null items in the foreach ($scope) loop.
    #Only add valid ScopeObj's
    if (($ScopeObj.InventoryList -ne $NULL) -and ($ScopeObj.TestFiles -ne $NULL)){
    $Final += $ScopeObj
    }
    This will only add populated values to the new collection.

  • Swap the foreach($test) and foreach($object)
    foreach($Test in $Tests)
    {
    Write-Verbose "Processing test file $Test"
    $TestName = Split-Path $Test -Leaf

      # Runs through each test file on the above objects in the currect scope
      foreach($Object in $Inventory) 
      {
    

This will give the currently desired format for reporting.

@brianbunke
Copy link
Contributor Author

Resolved in #115!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants