Never do this... PLEASE

I had been asked to make a couple of small amends to the functionality of an old legacy CF application, that had been created by someone who has long since left the company...

One of the pages in this application was responsible for displaying a list of all clients on the system, together with a count of the number of associated tasks. This page displays over 1700 rows (I know - paging wouldn't be a bad idea...)

At the bottom of the page, I was astounded to find this gem:

<cfoutput query="get_clients" >
   <!----- get count of tasks ----->
   <cfquery name="no_of_tasks" datasource="#request.dsn#">
          select count(*) as nooftasks from tasks where taskclientID = #clientID# and taskdeleted = 0
   </cfquery>
   <!----- get count of users ----->
   <cfquery name="no_of_users" datasource="#request.dsn#">
          select count(*) as noofusers from users where userclientID = #clientID#
   </cfquery>
</cfoutput>

This is terrible in several ways -

  • The queries are run once for each record within the outer query (get_clients) - 1708 times for each of the count queries
  • The output from these queries is never used!!!

I removed the above block of code - nothing broke as it wasn't being referenced, and the load time for the page went down from ~15 seconds to ~8.

I looked further up the page, and found that the queries I had deleted, had been copied from a location further up the page - where at least the calculated value was being referenced... or at least one of the values was being referenced - the HTML for displaying the other value had been commented out, without removing / commenting the 2nd query!

I removed the second query, which reduced the load time down to ~5 seconds.

So already, just by removing unused queries, we had a 300% improvement in performance.

Real world proof that parametrised SQL really does provide a measurable improvement in performance:

I wrapped the #clientID# value in the subquery in CFQUERYPARAM, and the page load time dropped from ~5 seconds to ~3 seconds...

Of course even with the 500% improvement in performance - I still didn't see the point of leaving this sub query in - when at most it would take me 15 min to build the aggregate in to the outer query.

So I did that, and page load dropped to < 700ms - over 20 times faster than the original page.

I calculated how many queries were being processed in the original page - it came to a mind blowing 6,833!!! - hardly surprising the page took a while to load!

I hope it is completely obvious to everyone why its a terrible idea to Nest CFQUERY calls within loops - performance of the application decreases logarithmically as the size of the outer data set increases - and its completely unnecessary, as the DB engine will be able to calculate the aggregates in the initial select statement, with virtually no measurable impact on performance - surprisingly enough, databases are actually pretty good at manipulating and doing calculations on data!!!

Comments
James Netherton's Gravatar Sadly, this is a problem that I've come across quite a few times, especially with code written by CF / web developer noobs.

What's worse is when you try and run the thing with debugging turned on - watch CF and your browser die while it tries to compute and display all of the query debug info :s
# Posted By James Netherton | 25/03/07 20:14
Dan Lancelot's Gravatar Heh -
I ran it to start with with the first 2 versions of http://coldfire.riaforge.org/ - and unsurprisingly the generated headers were too large... and the page failed to load.

I fixed it before version 0.0.3 came out - so i don't know how well that would cope - although I think trying to report on 6,833 db queries is too much for any debug tool :-|
# Posted By Dan Lancelot | 26/03/07 22:34
BlogCFC was created by Raymond Camden. This blog is running version 5.5.1, hosted by TalkWebSolutions.