Prevent repeated DB calls because of object oriented approach in MVC3 app

Question

We have a MVC3 application that we have created many small actions and views to handle placing the data wherever we need to. For instance if it was a blog and we wanted to show comments, we have a comment action and view and we can place that wherever we want, a user profile view, and blog post view, etc.

The problem this has caused is each small view or action needs to make a call, usually to the same service, multiple times per a page load because of all the other small views we have in our application. So on a very large page containing these small views, we could have 80+ sql calls with 40% of them being duplicates and then the page slows down. Current solution is to cache some data, and pass some data around in the ViewBag if we can so if you want like a user's profile, you check to see if its cache or the ViewBag if it isn't ask for it.

That feels really really dirty for a design pattern and the viewbag approach seems awful since it has to be passed from the top down. We've added some data into HttpCurrent.Items to make it per a request (instead of caching since that data can change) but there has to be some clean solution that doesn't feel wrong and is clean too?

EDIT

I've been asked to be more specific and while this is a internal business application I can't give away to much of the specifics.

So to put this into a software analogy. Lets compare this to facebook. Imagine this MVC app had an action for each facebook post, then under that action it has another action for the like button and number of comments, then another action for showing the top comments to the user. The way our app is designed we would get the current users profile in each action (thus like 4 times at the minimum in the above situation) and then the child action would get the parent wall post to verify that you have permission to see it. Now you can consider caching the calls to each security check, wall post, etc, but I feel like caching is for things that will be needed over the lifetime of the app, not just little pieces here and there to correct a mistake in how your application is architected.

Josh, can you be a little more specific about how your solution works. Are you using `Html.Action()` commands? How exactly are your commands duplicated? Can you provide any code, or pseudo-code to demonstrate the problem? Right now, we can only guess and make assumptions. By the way, what you describe has nothing to do with an "object oriented approach". It's more of a refactored approach. — Erik Funkenbusch, Dec 24 '11 at 04:22

score 2 · Answer 1 · answered Dec 21 '11 at 20:24

Are you able to replace any of your @Html.Action() calls with @Html.Partial() calls, passing in the model data instead of relying on an action method to get it from the db?

You could create a CompositeViewModel that contains your other ViewModels as properties. For example, it might contain a UserViewModel property, a BlogPostViewModel property, and a Collection<BlogComment> property.

In your action method that returns the container / master view, you can optimize the data access. It sounds like you already have a lot of the repeatable code abstracted through a service, but you didn't post any code so I'm not sure how DRY this approach would be.

But if you can do this without repeating a lot of code from your child actions, you can then use @Html.Partial("~/Path/to/view.cshtml", Model.UserViewModel) in your master view, and keep the child action method for other pages that don't have such a heavy load.

I went to try that approach, but the amount of refactoring we would have to do would be immense and some of our controllers would get out of hand. Since we use dependency injection, those controllers would have 10-20 servers passed into the constructor which is unacceptable really. That defeats the ability to keep the controller doing one and only one thing as it would have to do many. We are going to go AOP and implement caching as well to reduce and eliminate the extra calls. — Josh, Jan 12 '12 at 13:39

score 0 · Accepted Answer · edited May 23 '17 at 10:34

I see two potential places your code might be helped based on my understanding of your problem.

You have too many calls per page. In other words your division of work is too granular. You might be able to combine calls to your service by making objects that contain more information. If you have a comments object and an object that has aggregate data about comments, maybe combine them into one object/one call. Just a thought.
Caching more effectively. You said you're already trying to cache the data, but want a possibly better way to do this. On a recent project I worked on I used an AOP framework to do caching on WCF calls. It worked out really well for development, but was ultimately too slow in a heavy traffic production website.

The code would come out like this for a WCF call (roughly):

[Caching(300)]
Comment GetComment(int commentId);

You'd just put a decorator on the WCF call with a time interval and the AOP would take care of the rest as far as caching. Granted we also used an external caching framework (AppFabric) to store the results of the WCF calls.

Aspect Oriented Framework (AOP): http://en.wikipedia.org/wiki/Aspect-oriented_programming We used Unity for AOP: Enterprise Library Unity vs Other IoC Containers

I would strongly consider trying to cache the actual service calls though, so that you can call them to your hearts content.

I am marking your question as accepted since we do need to move to more AOP style of programming to limit/reduce our SQL calls. So instead of doing 5 calls, we will do one for the aspect of the program we need to work with. Also the following article talks some about it too. http://blogs.msdn.com/b/ricom/archive/2012/01/10/performance-and-design-guidelines-for-data-access-layers.aspx — Josh, Jan 12 '12 at 13:34
A while later, I was able to take this approach in our application. We had a new dashboard created from scratch. This dashboard pulls multiple items from multiple databases, including a data warehouse, our own application and our exchange server(for appointments). Total time to load IN DEV, under a second with no optimization. We were able to reuse most of our service layer and passed back down a larger object containing all the data we needed. If we wanted to use this code elsewhere we can just add it to another page and pass in the object or have it retrieve itself. This works great, THANKS! — Josh, Mar 04 '12 at 18:18

score 0 · Answer 3 · answered Dec 21 '11 at 18:11

The best thing to do is to create an ActionFilter that will create and teardown your persistence method. This will ensure that the most expensive part of data access (ie creating the connection) is limited to once per request.

public class SqlConnectionActionFilter : ActionFilterAttribute
    {
        public override void OnActionExecuting(ActionExecutingContext filterContext)
        {
            var sessionController = filterContext.Controller;

            if (filterContext.IsChildAction)
                return;

            //Create your SqlConnection here
        }

        public override void OnActionExecuted(ActionExecutedContext filterContext)
        {
            if (filterContext.IsChildAction)
                return;

            //Commit transaction & Teardown SqlConnection
        }
    }

The sql connection is one of the objects stored in the HttpCurrent.Items collection so it is only created once per a http request plus the usual backend connection pooling. — Josh, Dec 21 '11 at 20:28
While that's an interesting approach, it doesn't improve performance a great deal. Creating connections does not create a large performance problem, since connections are pooled by default anyways. You would gain just as much performance using Dependency Injection with a request scoped object liftime. — Erik Funkenbusch, Dec 24 '11 at 04:20

score 0 · Answer 4 · answered Dec 24 '11 at 03:52

0

The thing is: if you are doing the query 80 times, then you are hitting the db 80 times. Putting a request-scoped cache is the best solution. The most elegant way of implementing it is through AOP, so your code doesn't mind about that problem.

answered Dec 24 '11 at 03:52

Ivo

8,172
5
27
42

Prevent repeated DB calls because of object oriented approach in MVC3 app

4 Answers4