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!)
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 theirgivenName
,sn
, and/ormail
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 usingtry
/catch
blocks:You can use the null-conditional operators from C# 6 with the null-coalescing operator to simplify this to the following: