CODEWITHSUNDEEP
powershellloopsforeachactive-directory
blackworx
blackworx

Reputation: 567

Why does this Powershell ForEach loop get slower with each iteration?

My code is working as expected. I'm really just curious to know if anyone has any idea why what I describe below might be happening. That said, if anyone has any ideas for further optimising the routine, I'd gratefully accept them in the spirit of every day being a school day!

The script is querying all our domain controllers to get the most recent lastLogon attribute for all users in a particular OU. (I am doing this instead of using the lastLogonTimeStamp attribute because I need the absolute most recent logon as of the moment the script is run.)

During testing, to check the code was doing what I expected it to do, I added some console output (included in the snippet below).

When I did this I noticed that with each iteration of the SECOND ForEach ( $DC in $AllDCs ) loop, there was a noticeable pause before the nested loop wrote its first line of console output. The duration of the pause increased with each iteration of the outer loop, and the speed of the inner loop's subsequent output also dropped noticeably. Over the course of the run, looking at the output of a dozen or so DCs, I'd estimate the rate console lines were being written dropped by at least a factor of 4.

$AllDCs = Get-ADDomainController -Filter *

$AllRecords = @{}

ForEach ( $DC in $AllDCs ) {

    $UserList = Get-ADUser -filter * -Server $DC -SearchBase $ini.OUDN.NewStartsInternal -SearchScope OneLevel -Properties lastLogon
    $UserList = $UserList | Where { $_.SamAccountName -match $ini.RegEx.PayNo }

    $AllRecords.Add($DC.Hostname,$UserList)

}

$Logons = @{}

ForEach ( $DC in $AllDCs ) {                                   ; this loop is the one I'm talking about
    
    ForEach ( $User in $AllRecords.$($DC.HostName) ) {

        If ( $Logons.ContainsKey($User.SamAccountName) ) {     ;this line amended on advice from mklement0
            
            If ( $Logons.$($User.SamAccountName) -lt $User.lastLogon ) {
            
                $Logons.$($User.SamAccountName) = $User.lastLogon
                Write-Host "Updated $($User.SamAccountName) to $($User.lastLogon)" -ForegroundColor Green

            } Else {

                Write-Host "Ignored $($User.SamAccountName)"

            }

        } Else {

            $Logons.Add( $User.SamAccountName , $User.lastLogon )

            Write-Host "Created $($User.SamAccountName)" -ForegroundColor Yellow

        }

    }

}

I'm not really any under any time constraints here as we're only talking a couple hundred users and a dozen or so domain controllers. I've already reduced the runtime by a huge amount anyway. Previously it was looping through the users and querying every DC for every user one by one which, unsurprisingly, was taking far longer.

UPDATE:

I implemented mklement0's suggestion from the first comment below, and if anything the script is actually running more slowly than before. The delays between iterations of the outer loop are longer, and the output from the inner loop seems subject to random delays. On average I'd say the inner loop is getting through about 2 to 3 iterations per second, which to my mind is extremely slow for looping through data that is already held in local memory. I know PowerShell has a reputation for being slow, but this seems exceptional.

The script normally runs on a VM so I tested it on my own computer and it was a LOT slower, so this isn't a resource issue with the VM.

UPDATE 2:

I removed all the Write-Host commands and simply timed each iteration of the outer loop.

First of all, removing all the console writes increased performance dramatically, which I expected, although I didn't realise by how much. It easily cut the run time to a fifth of what it had been.

In terms of the loop times, the strange behaviour is still there. Out of twelve iterations, the first seven are done within 1 second, and getting through the final five takes about 35 seconds. This behaviour repeats more or less the same every time. There is nothing different about the hostnames of the final five DCs compared to the first seven that may be slowing down the hashtable lookup.

I'm very happy with the run time as it is now, but still utterly perplexed about this weird behaviour.

Upvotes: 1

Views: 1949

Answers (1)

Santiago Squarzon
Santiago Squarzon

Reputation: 61293

So, this is my take on your code, I didn't change many things but I have a feeling this should be a bit faster but I may be wrong.

Note, I'm using LDAPFilter = '(LastLogon=*)' instead of Filter = '*' because, if it's an attribute that is not replicated across the domain it will save time when querying each Domain Controller.

$AllDCs = Get-ADDomainController -Filter *
$logons = @{}

$params = @{
    LDAPFilter  = '(LastLogon=*)'
    Server      = ''
    SearchBase  = $ini.OUDN.NewStartsInternal
    SearchScope = 'OneLevel'
    Properties  = 'lastLogon'
}

foreach($DC in $AllDCs) {
    $params.Server = $DC
    $UserList = Get-ADUser @params
    
    foreach($user in $UserList) {
        if($user.samAccountName -notmatch $ini.RegEx.PayNo) {
            continue
        }

        if($logons[$user.samAccountName].LastLogon -lt $user.LastLogon) {
            # On first loop iteration should be always entering
            # this condition because
            # $null -lt [datetime]::MaxValue => True AND
            # $null -lt [datetime]::MinValue => True
            # Assuming $user.LastLogon is always a populated attribute
            # which also explains my LDAPFilter = '(LastLogon=*)' from before

            $logons[$user.samAccountName] = $user
        }
    }
}

Upvotes: 1

Related Questions