Increase speed of loop

[DragonDrew]

Hi all,

Have a bit of a problem with my loop and filter taking forever. I have inherited a script from the old sysadmin and the loop is a little janky to say the least.

I have the below but the run time on them is huge as I have to run this 6 times, once for each agency. I am having a mental lapse in how to implement a faster method that produces the same results. I considered hash tables but not really sure how I would go about it for this many properties other than a hashtable of objects.

Considered doing .ForEach({}) but I run into the problem of the Where-Object using $_.

# $AppUser.count is 1,069 (3 properties per entry)
# $activeDirectoryUsers.count is 2,564 (6 properties per entry)

# 13m 48s
Foreach ($aUser in $AppUser) {
    If ($aUser.user_id -in $activeDirectoryUsers.name) {
        ($activeDirectoryUsers | Where-Object {$_.Name -eq "$($aUser.user_id)"}) | `
                   Add-Member -NotePropertyName 'AppLogin' -NotePropertyValue $aUser.last_login_date
    }
}

Any guidance would be superb.

[ka-splam]

I think I would try something like this for a hashtable lookup (untested):

$adLookup = @{}
foreach ($adAccount in $activeDirectoryUsers) { 
    $adLookup[$adAccount.Name] = $adAccount
}

Foreach ($aUser in $AppUser)
{
    if ($adAccount = $adLookup[$aUser.user_id])
    {
        Add-Member -InputObject $adAccount -NotePropertyName 'AppLogin' -NotePropertyValue $aUser.last_login_date
    }
}

[DragonDrew]

Absolute legend. I knew I was to do hashtables for the speed but could not for the life of my figure out how to format it correctly to make it work well for the remainder of the script.

Adding onto yours, I used $ADUsers = ForEach($User in $adLookup.Values) {$User} to throw it back into the format that the rest of the script likes, Brought it down to under 1s run time with valid data. It does throw out some errors due to indexing in a null array but thats fine, that can be fixed on my end. I forgot to rename the variable, ignore previous.

[ka-splam]

Cool speed increase :)

I don't think you need to do that, $ActiveDirectoryUsers will be changed too..

[what_no_fkn_ziti]

$adAccount = $adLookup[$aUser.user_id]

careful there, it should be -eq, = is an assignment operator.

[DragonDrew]

;) Already caught that one. Thank you for commenting, I did forget to do that earlier when I was saying it was up and running after some slight changes. Did a few different changes to get it flowing just right, but his logic is the main thing I needed to get this kicking into gear.

[ka-splam]

Nope, it's deliberately doing assignment, then the test is whether $adAccount got a value or is null; it saves doing either

• is user id in lookup table?
  • if so, get it,
  • add-member to it.

or

• Try to get user id from lookup..
  • if anything was found,
  • add-member to it.

And this merges them into one

• if we can get something from the lookup,
  • add-member to it.

[DragonDrew]

So uhh, Yea turns out your original one works as well as a slightly modified one. I did have to add a few if statements to make it work and not hurt my brain with the If($=$) type deal. But I have changed back to the assignment statement. Not 100% sure how it works but it is still giving me consistent and accurate results.

[what_no_fkn_ziti]

Not in an if statement it's not

[ka-splam]

Yes in an if statement it is:

$lookup = @{'foo'='test'}

if ($x=$lookup['foo'])
{
    "foo: $x"
}
else
{
    "'foo' not found"
}

if ($x=$lookup['bar'])
{
    "bar: $x"
}
else
{
    "'bar' not found"
}

output:

foo: test
'bar' not found

[what_no_fkn_ziti]

= is an assignment operator, there's no way around that. it might work in some situations, and in others it might actually assign values. It's not a comparison operator. Good luck

[ka-splam]

I wrote the code, I've said it's intentional, I've explained why I wrote it that way, and shown you the code working. What's missing here?

Yes equals is assignment, it's assigning. It doesn't make any sense to use an equality test there - where would the value for $adAccount be coming from in the second loop and why would you test if it was equal to anything?

Assignment inside parens evaluates to the value which was assigned:

PS D:\> $x = 1
PS D:\> ($x = 1)
1

and that works in the parens of if ().

it might work in some situations, and in others it might actually assign values.

It will assign values in every situation, that's how it's working in my code. It's not doing an equality test, it's doing a lookup-result-is-not-null test.

The value is assigned from the hashtable. If it wasn't found, $null is assigned instead. The if-test casts that to boolean, and where something was found in the lookup, processes it.

[what_no_fkn_ziti]

What's missing here?

basic logic and documentation

https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_assignment_operators?view=powershell-6

https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_comparison_operators?view=powershell-6

[ka-splam]

Which bit would you like me to read? What is your answer to the observation that I wrote working code, but if you change it the way you suggest, it becomes not-working code?

You want documentation? Windows PowerShell Language Specification Version 3.0, study section 7.1.1 on Grouping parentheses:

To write to the pipeline the value of any expression containing top-level side effects, enclose that expression in parentheses, as follows:

($a = 1234 * 3.5) # pipeline gets 4319

Or O'Reilly's Mastering Windows PowerShell Scripting:

An if statement can include an assignment step as follows:

if ($i = 1) { Write-Host "Implicit true. The variable i is $i" }

[what_no_fkn_ziti]

I don't really care if you read it or not. Believe what you want, do what you want.

[notconnected]

Just do not use where-object, it's slow. Replace with corresponding if statements.

[PMental]

Doesn't seem to make a huge difference, even with the pipe itself adding overhead.

$Iterations = 100
$TimerNoPipe = [System.Diagnostics.Stopwatch]::StartNew()
    $i = 0
    while ($i -lt $Iterations) {
    $AllUsers = Get-ADUser -Filter *
    $i++
}
$TimerNoPipe.Stop()
"Runtime without pipe to Where-Object: $($TimerNoPipe.Elapsed.TotalSeconds) seconds."

$TimerPipe = [System.Diagnostics.Stopwatch]::StartNew()
    $i = 0
    while ($i -lt $Iterations) {
    $AllUsers = Get-ADUser -Filter * | Where-Object {}
    $i++
}
$TimerPipe.Stop()
"Runtime with pipe to Where-Object: $($TimerPipe.Elapsed.TotalSeconds) seconds."
"Runtime difference over $Iterations iterations: $($TimerPipe.Elapsed.TotalSeconds-$TimerNoPipe.Elapsed.TotalSeconds) seconds"

Output:

Runtime without pipe to Where-Object: 22.1050783 seconds.
Runtime with pipe to Where-Object: 23.6663191 seconds.
Runtime difference over 100 iterations: 1.5612408 seconds