alex alex - 3 months ago 8
C# Question

Improve code performance

I have a cron-job method which constructs a user's story feed based on user's featured stories, followed categories and followed users.

The final feed is added in the below database table in correct order:

UserFeed table:

Uid     StoryListIds

1         3, 23, 45, 3, 6, 234, .....

2         3, 23, 45, 6, 87, 44, .....

3         3, 23, 45, 32, 4, 62, .....

The method is below and contains comments.

The code:

public void ConstructUserFeed()
{
try
{
//get all user ids
var userIds = userBL.GetIds();
//get all featured stories
var featStories = storyBL.GetFeaturedStories();

if (userIds != null && userIds.Count > 0)
{
foreach (var userId in userIds)
{
//integer List to store the stories ids in correct order
List<int> storyIdsLst = new List<int>();
//integer List for excluding duplicates
List<int> exceptStoryIds = new List<int>();
//user feed List to store the stories
var userFeed = new List<UserFeedDTO>();
//string to store the id list so we can add it later in db
string storyIds = "";

if (featStories != null && featStories.Count > 0)
{
foreach (var featStory in featStories)
{
//first add all the ids of featured stories except own user stories, ordered by date
if (featStory.authorId != userId)
{
storyIdsLst.Add(featStory.id);
exceptStoryIds.Add(featStory.id);
}
}
}

//get user's followed categories ids
var followedCategoryIds = userCategoriesBL.GetFollowedCategoryIds(userId);

if (followedCategoryIds != null && followedCategoryIds.Count > 0)
{
foreach (var categoryId in followedCategoryIds)
{
//get the user's 5 latest stories for every followed category
//except own stories and previous stories
var storiesByCateg = storyBL.GetByCategory(5, categoryId, userId, exceptStoryIds);

if (storiesByCateg != null && storiesByCateg.Count > 0)
{
foreach (var storyByCateg in storiesByCateg)
{
userFeed.Add(storyByCateg);
exceptStoryIds.Add(storyByCateg.id);
}
}
}
}

//get user's followed users ids
var followedUserIds = userFollowersBL.GetFollowedUserIds(userId);

if (followedUserIds != null && followedUserIds.Count > 0)
{
foreach (var followedId in followedUserIds)
{
//get the user's 5 latest stories for every followed user
//except own stories and previous stories
var storiesByFollowedUsers = storyBL.GetByFollowedUser(5, followedId, userId, exceptStoryIds);

if (storiesByFollowedUsers != null && storiesByFollowedUsers.Count > 0)
{
foreach (var storyByFollowedUsers in storiesByFollowedUsers)
{
userFeed.Add(storyByFollowedUsers);
}
}
}
}

// order the stories by date
userFeed = userFeed.OrderByDescending(story => story.dateAdded).ToList();

if (userFeed != null && userFeed.Count > 0)
{
foreach (var story in userFeed)
{
//add the story ids after the featured story ids
storyIdsLst.Add(story.id);
}
}

//comma separated list of story ids as string so we can store it in db
storyIds = string.Join(",", storyIdsLst.Select(n => n.ToString()).ToArray());

//create the UserFeed model
UserFeed userFeedModel = new UserFeed();
userFeedModel.userId = userId;
userFeedModel.storyListId = storyIds;
userFeedModel.lastUpdateTime = DateTime.Now;

userFeedBL.AddOrUpdateUserFeed(userFeedModel);
}
uof.Save();
}
}

catch (Exception ex)
{
Console.WriteLine("Error occurred in processing job. Error : {0}", ex.Message);
}
}


The above method takes ~35 seconds to complete for 30 users.

Q: How can I improve my code and performance?

Answer

It's hard to say exactly what's causing it to execute so slowly, for that I recommend profiling your question with SQL Server Profiler. Check that your queries is properly asked and doesn't do anything unnecessary.

After that, I would consider asking fewer questions. Since you're doing it in a loop you may benefit from doing fewer, but heavier questions. For example (assuming userFollowersBL.* is querying the DB):

var followedCategoryIds = userCategoriesBL.GetFollowedCategoryIds(userId);

I assume that the signature is something like:

IEnumerable<int> GetFollowedCategoryIds(userId);

Consider changing it to:

IDictionary<int, IEnumerable<int>> GetFollowedCategoryIds(IEnumerable<int> userIds);

Then you'll have a dictionary with the userids and each of their followedCategoryIds in memory before you start your foreach. This way you can send all your result from userBL.GetIds() in one query. It may speed up performance to do it once, instead of 30 times. The same thing goes for userFollowersBL.GetFollowedUserIds(userId).

Now you have decreased the number of queries to you DB with approx 58 times.

public void ConstructUserFeed()
        {
            try
            {
                //get all user ids
                var userIds = userBL.GetIds();
                //get all featured stories
                var featStories = storyBL.GetFeaturedStories();

// Fetch all in one query.
IDictionary<int,IEnumerable<int>> allFollowedCategoryIds= userCategoriesBL.GetFollowedCategoryIds(userIds);
// Fetch all in one query
IDictionary<int,IEnumerable<int>> allUserFollowers = userFollowersBL.GetFollowedUserIds(userIds);

                if (userIds != null && userIds.Count > 0)
                {
                    foreach (var userId in userIds)
                    {
                        //integer List to store the stories ids in correct order
                        List<int> storyIdsLst = new List<int>();
                        //integer List for excluding duplicates
                        List<int> exceptStoryIds = new List<int>();
                        //user feed List to store the stories
                        var userFeed = new List<UserFeedDTO>();
                        //string to store the id list so we can add it later in db
                        string storyIds = "";

                        if (featStories != null && featStories.Count > 0)
                        {
                            foreach (var featStory in featStories)
                            {
                                //first add all the ids of featured stories except own user stories, ordered by date
                                if (featStory.authorId != userId)
                                {
                                    storyIdsLst.Add(featStory.id);
                                    exceptStoryIds.Add(featStory.id);
                                }
                            }
                        }

                        //get user's followed categories ids
                        var followedCategoryIds = allFollowedCategoryIds[userId]

                        if (followedCategoryIds != null && followedCategoryIds.Count > 0)
                        {
                            foreach (var categoryId in followedCategoryIds)
                            {
                                //get the user's 5 latest stories for every followed category
                                //except own stories and previous stories
                                var storiesByCateg = storyBL.GetByCategory(5, categoryId, userId, exceptStoryIds);

                                if (storiesByCateg != null && storiesByCateg.Count > 0)
                                {
                                    foreach (var storyByCateg in storiesByCateg)
                                    {
                                        userFeed.Add(storyByCateg);
                                        exceptStoryIds.Add(storyByCateg.id);
                                    }
                                }
                            }
                        }

                        //get user's followed users ids
                        var followedUserIds = allUserFollowers[userId];

                        if (followedUserIds != null && followedUserIds.Count > 0)
                        {
                            foreach (var followedId in followedUserIds)
                            {
                                //get the user's 5 latest stories for every followed user
                                //except own stories and previous stories
                                var storiesByFollowedUsers = storyBL.GetByFollowedUser(5, followedId, userId, exceptStoryIds);

                                if (storiesByFollowedUsers != null && storiesByFollowedUsers.Count > 0)
                                {
                                    foreach (var storyByFollowedUsers in storiesByFollowedUsers)
                                    {
                                        userFeed.Add(storyByFollowedUsers);
                                    }
                                }
                            }
                        }

                        // order the stories by date
                        userFeed = userFeed.OrderByDescending(story => story.dateAdded).ToList(); 

                        if (userFeed != null && userFeed.Count > 0)
                        {
                            foreach (var story in userFeed)
                            {
                                //add the story ids after the featured story ids
                                storyIdsLst.Add(story.id);
                            }
                        }

                        //comma separated list of story ids as string so we can store it in db 
                        storyIds = string.Join(",", storyIdsLst.Select(n => n.ToString()).ToArray());

                        //create the UserFeed model
                        UserFeed userFeedModel = new UserFeed();
                        userFeedModel.userId = userId;
                        userFeedModel.storyListId = storyIds;
                        userFeedModel.lastUpdateTime = DateTime.Now;

                        userFeedBL.AddOrUpdateUserFeed(userFeedModel);
                    }
                    uof.Save();
                }
            }

            catch (Exception ex)
            {
                Console.WriteLine("Error occurred in processing job. Error : {0}", ex.Message);
            }
        }

But honestly, this is mostly speculations and may differ from time to time. In my experience you tend to benefit from asking for more data once, than the same data several times.