Why is my directory search taking so long?

Question

I've been troubleshooting performance in a REST API I've built that, among other things, returns a list of users from Active Directory based on a search term that is provided. Based on some logging that I've built in for test purposes, I can see that the whole process of fetching the settings (e.g. LDAP search information) and retrieving all of the search results takes less than a second:

30/08/2017 3:37:58 PM | Getting search results.
30/08/2017 3:37:58 PM | Retrieving default settings
30/08/2017 3:37:58 PM | Default settings retrieved. Creating directoryEntry
30/08/2017 3:37:58 PM | Search retrieved.
30/08/2017 3:37:58 PM | Iterating through search results.
30/08/2017 3:38:16 PM | Search results iteration complete.

However, as you can see iterating through these search results and populating my list of users is taking 18 seconds. This is my code:

SearchResultCollection resultList = new DirectorySearcher(CreateDirectoryEntry())
{
    Filter = ("(&(objectClass=user) (cn=*" + SearchTerm + "*))"),
    PropertiesToLoad =
    {
        "givenName",
        "sn",
        "sAMAccountName",
        "mail"
    }
}.FindAll();

foreach (SearchResult result in resultList)
{
    ADUser thisUser = new ADUser();

    try
    {
        thisUser.Firstname = result.Properties["givenName"][0].ToString();
    }
    catch
    {
        thisUser.Firstname = "Firstname not found";
    }
    try
    {
        thisUser.Lastname = result.Properties["sn"][0].ToString();
    }
    catch
    {
        thisUser.Lastname = "Lastname not found";
    }
    try
    {
        thisUser.EmailAddress = result.Properties["mail"][0].ToString();
    }
    catch
    {
        thisUser.EmailAddress = "Email address not found";
    }

    UserList.Add(thisUser);
}

It's pretty vanilla and not doing anything fancy. Any idea why this would be so slow, or any suggestions as to what I could do differently to speed this up?

UPDATE

Based on comments and answers I have removed the null checking from the code. So now it looks like this:

foreach (SearchResult result in resultList)
{
    ADUser thisUser = new ADUser();

    thisUser.Firstname = result.Properties["givenName"][0].ToString();
    thisUser.Lastname = result.Properties["sn"][0].ToString();
    thisUser.EmailAddress = result.Properties["mail"][0].ToString();

    UserList.Add(thisUser);
}

This has not improved the performance. I can see this loop is still taking around 18 seconds, even when there is only one result returned. (It's also demonstrated that crappy data in my AD means I need this null check!)

what if you will do if(result.Properties["givenName"][0] != null) — Leon Barkan, Aug 30 '17 at 06:08
if it's null enter your "exception" message without try,catch and so on — Leon Barkan, Aug 30 '17 at 06:09
is SearchResult a `System.DirectoryServices.SearchResult` or is it your own custom type? if it is the framwork type, please update your question to show how you build your `DirectorySearcher` object. — Scott Chamberlain, Aug 30 '17 at 06:10
@ScottChamberlain I believe SearchResult is lazy loaded, and it is most likely that he uses the type you referred. — Ghasan غسان, Aug 30 '17 at 06:14
@GhasanAl-Sakkaf i agree, if the OP did not include the columns he is checking information for to the `DirectorySearcher.PropertiesToLoad` collection each request will require a round trip to the AD server due to the lazy loading. — Scott Chamberlain, Aug 30 '17 at 06:17
@ScottChamberlain yes I'm using the default type and yes I have already restricted the PropertiesToLoad. Updating Q now. — Matt G, Aug 30 '17 at 06:22
What is returned by `CreateDirectoryEntry()`? If you explicitly specify to connect to a local domain controller by setting the `searchRoot` parameter/`SearchRoot` property of your `DirectorySearcher` to something like `"LDAP://" + dcIPOrName` does that improve anything? — Lance U. Matthews, Aug 30 '17 at 15:38
@BACON I've set the searchRoot to a specific OU I'm searching for results from. What's dcIPOrName? — Matt G, Aug 30 '17 at 20:32
@MattG `dcIPOrName` would be a variable containing the IP address or host name of a domain controller to connect to. My point is to ensure that the search is being performed by a domain controller on your local network and not one on the other side of the planet (not knowing what your forest looks like). Also, perhaps a Wireshark packet capture would give some insights into why the query is taking so long. — Lance U. Matthews, Aug 30 '17 at 20:50

Lance U. Matthews · Accepted Answer · 2017-08-30T06:27:20.410

You are relying on exceptions when setting the properties of thisUser when, depending on your directory, it might not be all that exceptional for a user to not have their givenName, sn, and/or mail attributes populated. If you have a long list of search results all these exceptions can add up. Consider checking if the desired property exists instead of using try/catch blocks:

thisUser.Firstname = result.Properties.Contains("givenName")
    ? result.Properties["givenName"][0].ToString()
    : "Firstname not found";
thisUser.Lastname = result.Properties.Contains("sn")
    ? result.Properties["sn"][0].ToString()
    : "Lastname not found";
thisUser.EmailAddress = result.Properties.Contains("mail")
    ? result.Properties["mail"][0].ToString()
    : "Email address not found";

You can use the null-conditional operators from C# 6 with the null-coalescing operator to simplify this to the following:

thisUser.Firstname = result.Properties["givenName"]?[0]?.ToString() ?? "Firstname not found";
thisUser.Lastname = result.Properties["sn"]?[0]?.ToString() ?? "Lastname not found";
thisUser.EmailAddress = result.Properties["mail"]?[0]?.ToString() ?? "Email address not found";

Thanks for this. I've removed all of the error checking from the code altogether and confirmed it does not have a significant impact on performance. Updating question to reflect this. — Matt G, Aug 30 '17 at 06:50
Also, this is much more elegant than the cack-handed way I did it, so thanks! I'll be incorporating this for the null check when I get past the performance issue. — Matt G, Aug 30 '17 at 20:31
Marking as answer. Performance is now vastly improved (<1s). The null-conditional operator took as long as the try/catch, but using the first method you've suggested seems to have nailed it. — Matt G, Aug 30 '17 at 23:52

Leon Barkan · Answer 2 · 2017-08-30T06:19:04.370

you can try something like that:

foreach (SearchResult result in resultList)
            {
                ADUser thisUser = new ADUser();
                if(result.Properties["givenName"][0] != null)
                    thisUser.Firstname = result.Properties["givenName"][0].ToString();
                else
                    thisUser.Firstname = "Firstname not found";

                if(thisUser.Lastname = result.Properties["sn"][0] != null)
                    thisUser.Lastname = result.Properties["sn"][0].ToString();
                else
                    thisUser.Lastname = "Lastname not found";

                if(result.Properties["mail"][0] != null)
                    thisUser.EmailAddress = result.Properties["mail"][0].ToString();
                else
                    thisUser.EmailAddress = "Email address not found";

                UserList.Add(thisUser);
            }

Why is my directory search taking so long?

2 Answers2