Friday, November 28, 2008

ASP.NET AJAX: Replacement CSS add/remove functions

I came across a major performance issue in the Microsoft ASP.NET AJAX client library (JavaScript library) today.

Because the nature of the issue is engrained in the client library code, the only way around it was to completely abandon use of the functions that add, remove, and test for the existence of class names assigned to DOM elements, and develop my own.

Specifically, the following functions are involved/affected:

  • Sys.UI.DomElement.addCssClass
  • Sys.UI.DomElement.removeCssClass
  • Sys.UI.DomElement.containsCssClass
  • Sys.UI.DomElement.toggleCssClass

In program code that changes only a few classes here and there, nobody would ever notice performance problems, but when iterating over a large set of DOM elements .... well, that's a different story.

In the process of investigating the performance issues, I took a look at the code behind those functions.  I didn't do a bunch of stand-alone, isolated tests to prove my theory (because frankly I don't have the time to do it!), but I believe the problem lies in the containsCssClass function. 

Internally, almost every other CSS function calls containsCssClass, so because containsCssClass has a performance issue, it ripples through the other functions as well.

The containsCssClass function does a lot of unncessary work just to find out if a particular element contains a specific class name.  It performs a JavaScript split call on the string, then goes through the newly-created array one element at a time to look for a match.  But in doing that, it makes various function calls, performs redundant typeof checks — and performs a case-sensitive test on the class name!

So if you perform many addCssClass calls in a loop, you're actually performing lots and lots of expensive calls involving dynamically-created strings and arrays, as well as unnecessary type- and bounds-checking, hurting not only performance, but memory usage.

The case-sensitive matching is also a problem for me, because CSS class names are not case-sensitive by definition.  The class name "MyClass" should be able to match with "myclass", but it would not match when using addCssClass, so you could end up having multiple copies of the same class name in a DOM element.

removeCssClass also suffers from case-sensitive matching.  Not good.

Another potential problem I found when looking at the Microsoft code is that it does not account for the possibility that a class name appears in the element more than once.  It may be a mistake on the part of a developer to do that, but it happens, and it should be accounted for.

For example, given the class name "customer gold vip gold", if you execute the function call Sys.UI.DomElement.removeCssClass(element, "gold"), the new class name will become "customer vip gold".  See how the class "gold" is still included in the string, even though you removed it?  That's because only the first instance of "gold" was removed.

Last, I hope Microsoft does something about those lengthy namespaces.  Having the CSS class manipulation functions nested 4-levels deep in a namespace hierarchy is just plain inefficient and verbose.  Maybe they will fix that problem in .NET 4.0.

Function Rewrites

Someday — probably not too distant in the future — I'll be converting much of my code to take advantage of jQuery, but until that time, the solution for this type of problem is to rewrite the code myself.

The end result of my efforts were very easy to measure:  a loop that modifies many class names, which was taking 15 seconds to run, now takes 1 second to complete. 

I also threw in an extra function that allows class names to be removed with a regular expression (Regex) instead of a fixed string.  (This is useful in cases such as when you want multiple class names removed.  It can now be accomplished with one function call.)

My new program code for CSS manipulation is shown below. 

In the code, I have wrapped the functions in a global object called "DOM", so calling the functions is accomplished like this:  DOM.addCssClass(...).

Of course, the global object ("DOM") can be called anything.  I personally like using short names for heavily-used global objects, because it keeps code tighter and more legible.

var DOM = {

    addCssClass: function (element, className, noTest) {
        if ((noTest) || (!DOM.containsCssClass(element, className))) {
            element.className = (element.className + " " + className).trim();
        }
    },

    containsCssClass: function (element, className) {
        return ((" " + element.className.toLowerCase() + " ").indexOf(" " + className.toLowerCase() + " ") >= 0);
    },

    removeCssClass: function (element, className, noTest) {
        DOM.removeCssClassRegex(element, new RegExp("(^| )" + className + "($| )", "gi"), noTest);
    },

    removeCssClassRegex: function (element, classRegex, noTest) {
        if ((noTest) || (classRegex.test(element.className))) {
            element.className = element.className.replace(classRegex, " ").replace(/\s{2,}/g, " ").trim();
        }
    },

    toggleCssClass: function (element, className) {
        if (DOM.containsCssClass(element, className)) {
            DOM.removeCssClass(element, className, true);
            return false;
        }
        else {
            DOM.addCssClass(element, className, true);
            return true;
        }
    }
}

The above code is also available for download.

Designed for Speed

A few key points about the design of the new CSS class name manipulation code:

  • There are built-in ways to ensure that the testing of values and class names is only done once per call.  In addCssClass, setting the noTest argument to true signifies that the class name definitely does not exist in the element, and the containsCssClass function call is skipped.  In removeCssClass, the noTest argument set to true indicates that the class name does exist, without the need to test again.
  • containsCssClass does its testing using a simple indexOf string function, which is very fast.  It also performs a case-insensitive test, which, as stated above, is an important omission on Microsoft's part.
  • removeCssClass now removes all instances of the given class name, not just the first one.  It accomplishes the removal with a very simple regular expression (Regex), which is also efficiently used to test for the existence of the class name (if noTest is not true).  Sometimes an efficient regular expression pattern can outperform a bunch of string manipulation, depending on what you're trying to do.
  • A "bonus" function was added, removeCssClassRegex, which can remove class names by specifying a regular expression instead of a fixed string.  For example, to remove both "gold" and "vip" class names from an element, you would call: DOM.removeCssClassRegex(element, /(^| )(gold|vip)($| )/gi);

Are there ways of making the code even faster?  Possibly.  But probably not by the orders of magnitude that I've just increased it by.  Also, one has to balance performance with clarity of the code, which can sometimes suffer when the only objective is performance.

Generally speaking I like what Microsoft has done with its ASP.NET AJAX client library.  It's a pretty amazing achievement.

At the same time, it's always good to recognize the parts that need improvement.  Microsoft is doing a good job of listening to its community of developers these days, so I'll continue trying to point out things like this whenever I can, in the hope that they get noticed and fixed.

3 Comments:

At 1:24 PM, Ben Amada said...

Nice improvements. 15 seconds down to 1 second is terrific. I wonder if there would be any measureable difference if your containsCssClass() function used a regexp too.

At 4:11 PM, Todd said...

Thanks for the comment Ben.

Using a RegExp in containsCssClass would probably slow it down a little, not only in the execution speed of the test, but also when the JavaScript parser dynamically creates and compiles the regular expression.

When I am building a string-manipulation function strictly for speed, those are the types of things I try to anticipate. Regular expressions can be a double-edged sword, in that they can be incredibly flexible and even easy to use (when you've grown accustomed to them), but on the other hand there is overhead created both when they are first created and then when they're executed.

The fastest string manipulation possble is when you're "peeking" into a string at a certain character position and looking at what's there. There are a lot of technical reasons for that, mainly to do with how memory is manipulated with JavaScript strings, but that's why the containsCssClass is written that way.

The removeCssClass was written with a regular expression because it is doing a much more complex operation, and there isn't much of a performance difference between running one RegExp and executing the many steps that would be necessary without a RegExp.

The additional complexity comes from the need to remove all instances of a class name, the need to consolidate strings, and be case-insensitive without disturbing the rest of the string.

There is a way to make the function even faster, and I'll probably update the code a bit. In the removeCssClass I will be adding a test to see if a space was present in the string. If there is no space, that means it is only one class name, and a simple test for equality can be used, eliminating the need for any RegExp at all. If there's a space, then the RegExp can be used.

At 8:16 PM, Bleroy said...

Thanks for the feedback. I'll be sending this to the Ajax team. One thing that looks weird though is that you'd have type checking in there. If you run the scripts in release mode, no such check should happen.

Post a Comment

<< Home