1

I have to highlight some words in a div[contenteditable=true] without modifying its html so I am duplicating the div and positioning the copy right behind the original and then apply some span around the words I need to higlight.

The original div may modify its class/attributes at any time so I want to keep the copy updated with any css changes and to do this I have this function which gets called everytime I call my highlighting function.

My problem is that this function takes to much time to execute, around 60 ms whereas the rest of my script takes about 1-4 ms.

What am I doing wrong here or how can I speed it up?

function positionOutputWindow(original,copy)
{
    //console.log("positionOutputWindow");
    //var start = new Date().getTime();
    if ( 
        !original ||
        !copy ||
        original.attr('id') == undefined || 
        copy.attr('id') == undefined
        ) { return; }

    var original_obj = original.get(0);
    var copy_obj = copy.get(0);
    var offset = original.offset();

    copy.offset(offset);
    copy.css({
        'position': 'absolute',
        'z-index': '2',
        'color': 'transparent',
        'flood-color': 'transparent',
        '-webkit-text-fill-color': 'transparent',
        //'overflow': 'hidden',
        'outline': 'solid 0px red'/*,
        'width': original.width() + 'px',
        'height': original.height() + 'px'*/
    });
    copy.css("background",original.css("background"));
    original.css({
        //'overflow': 'hidden',
        'background': 'transparent',
        'position': 'relative',
        'z-index': '3',
        'outline': 'solid 0px green'
    });
    copy.width(original.width());
    copy.height(original.height());

    if ( original.get(0).nodeName == "INPUT" || original.get(0).nodeName == "TEXTAREA" )
    { 
        copy.width(original_obj.scrollWidth); 
        copy.offset({ top: offset.top, left: offset.left - original_obj.scrollLeft });
    }

    copy.offset({ top: offset.top, left: offset.left });
    copy.find('*').css('color','transparent');
    //console.log("runtime position window " + (new Date().getTime() - start));
}
nucka
  • 559
  • 5
  • 12
  • 2
    Is the performance causing a problem? If not, then you may be optimizing before you need to. It would be more worthwhile to make sure that the function is **correct** and **maintainable**. – voithos Oct 08 '13 at 15:35
  • Try putting the css in classes, and just swap the class names instead of setting each css property like you are doing. The css seems like it would cause the most slowdown. Not sure you can expect much faster than that though for DOM manipulation. – Andrew Grothe Oct 08 '13 at 15:36
  • my diagnosis: too much jQuery. – lukaleli Oct 08 '13 at 15:37
  • yes it's causing problems because during those 60ms the website is not responsive when a person types – nucka Oct 08 '13 at 15:39
  • 1
    You're executing all this every time a person types? – Mike Robinson Oct 08 '13 at 15:40
  • 1
    60ms means you can execute your script fully 16 times every second. Is that really necessary? If so, then realize that each call to change the DOM is expensive - it is often better to do only one change even if it is a large "replace this whole div with this one" command. So if you must optimize, replace the functionality with figuring out what the final div will contain, and then replace the HTML in one fell swoop instead of one div at a time. Redraws/reflows tend to be expensive. – BrianH Oct 08 '13 at 15:41
  • Also, if your script is executing every time a letter is typed or some other common event is used, that's probably asking too much of the poor ole browser; set up at least a minimum delay so you aren't running it constantly. You just set your logic so that it does nothing if it was updated within n seconds, regardless of user activity, and then after n seconds it only updates if there is work to do. Sometimes doing less work is infinitely better than doing your work faster. – BrianH Oct 08 '13 at 15:45
  • Yes I'm doing the minimum delay stuff but experimenting with the amount of delay didn't really change a lot so I guess maybe the responsiveness issue is not due to this function :/ – nucka Oct 09 '13 at 07:08

1 Answers1

1

First, you should read up on reflows:

When does reflow happen in a DOM environment?

I see an awful lot of DOM manipulation in this function. Multiple calls to offset, css, width, and height. If you want to improve performance, try combining all of those into a single call.

Second, this line really stands out:

copy.find('*').css('color','transparent');

This could contain a decent amount of elements. You're asking javascript to find every one and apply a transparent color every single time the user hits a key. Surely there's a better way? Not to mention transparency can be fairly CPU intensive.

Finally, why do you need to call this function every time you type? Is it really necessary? What are you trying achieve?

Community
  • 1
  • 1
Mike Robinson
  • 24,971
  • 8
  • 61
  • 83
  • After testing the times for each line of code the result is that the .offset command and the width and height are the ones that take a lot more time than the rest, the rest takes about 0-1 ms total... Also it only takes a lot of time for the first of those commands to execute. Is it possible that the reason for this is that I am not caching the DOM object and looking for it each time before calling my positionOutputWindow, thus the first time JS tries to access a property of it it takes longer that usual? – nucka Oct 09 '13 at 07:00
  • @nucka No it's not the caching. It's the fact that calling those functions cause the browser to redraw (aka rerender the entire screen). Since you're calling them multiple times, you're forcing the browser to recalculate it's entire layout multiple times in the same function call. – Mike Robinson Oct 09 '13 at 14:11
  • ok, so if I want to avoid this I should avoid calling this functions (which I can maybe do for some of them but not all)? Or is there maybe a way to tell the browser not to redraw the page until my function has done its job so it will redraw it only once? thank you for helping me understand – nucka Oct 10 '13 at 13:02