Javascript closure variables performance

Question

I have a function which binds different form inputs

function bindInputs() {
    $(".inputContainer").each(function(i){
        var inputContainer = $(this),
            input = $("input.input", inputContainer),
            inputType = inputContainer.attr("data-inputType"),
            input_Id = inputContainer.attr("id").replace("inputContainer_", "");


        if(inputType == "TextEditor") {
            input.unbind("change").bind("change", function() {
                inputContainer.removeClass("nullValue");
                var value = input.val();
                saveInputValue(input_Id, value);
            });

            return true;
        }

        if(inputType == "NumericEditor") {
            input.numeric({ allow: "." });
            input.unbind("change").bind("change", function() {
                inputContainer.removeClass("nullValue");
                var value = getNumericValue(input.val());
                saveInputValue(input_Id, value);
            });
        }

        // so on
    });
};

Is this function creating memory leaks? The thing i am worried about is that i keep all the shared variables on top and use them inside "change" callback functions.

Would make a difference if i re-calculate the shared variables on the callback functions?

if(inputType == "TextEditor") {
    input.unbind("change").bind("change", function() {
        var elem = $(this),
            inputContainer = elem.closest(".inputContainer"),
            input_Id = inputContainer.attr("id").replace("inputContainer_", "");

        inputContainer.removeClass("nullValue");
        var value = input.val();
        saveInputValue(input_Id, value);
    });

    return true;
}

Your code won't run, it got extra double quotes causing syntax error (in the line input = $("input.input", inputContainer")). For start, please post valid code. — user447356
– user447356, Commented Dec 12, 2012 at 9:59
Sorry but this is just an example to explain the question, not a code that should compile. And yes, i had a typing mistake, i will fix it. You are picky! — Catalin
– Catalin, Commented Dec 12, 2012 at 10:01
@ShadowWizard That was fixed pretty quickly...and then fixed again to be correct. Typos are often made on SO, it's a mistake, no big deal... — Ian
– Ian, Commented Dec 12, 2012 at 10:04
@Ian since those things might be the source of the problems fixing them sometimes render the whole question pointless. Might not be the case on this specific question, but better play it safe. — user447356
– user447356, Commented Dec 12, 2012 at 10:43

T.J. Crowder · Accepted Answer · 2012-12-12 10:12:56Z

6

It's not creating leaks. It is preserving those variables in memory for as long as those functions are in memory.

Whether you want to do that is up to you, it's a trade-off. For a change handler, the overhead of re-querying the DOM is minimal so you might go with your second example, although the actual memory impact of what you're retaining in that example is fairly minimal. In a mousemove handler, you'd probably go the other way, because mousemove handlers need to do their work very quickly.

Re your question in the comments below:

Saying that i will chose the second approach, how i will prevent the browser to save the top variables? I set them to null at the end of the function?

If you're going to make the functions not rely on anything in the bindInputs function, define them outside of it entirely. Then you're not creating any closures at all over the context of the call to bindInputs, and that context can be GC'd (along with any variables it contains). Setting variables to null or undefined just sets them to null or undefined, it doesn't get rid of them. (Although at that point, the context containing them is pretty small. Just three or four vars that don't reference anything in particular.)

Here's what that might look like:

function bindInputs() {
    $(".inputContainer").each(function(i){
        var inputContainer = $(this),
            input = $("input.input", inputContainer),
            inputType = inputContainer.attr("data-inputType");


        if(inputType == "TextEditor") {
            input.unbind("change").bind("change", handleTextEditorChange);

            return true;
        }

        if(inputType == "NumericEditor") {
            input.numeric({ allow: "." });
            input.unbind("change").bind("change", handleNumericEditorChange);
        }

        // so on
    });
}

function handleTextEditorChange() {
    // ...implementation...
}

function handleNumericEditorChange() {
    // ...implementation...
}

edited Dec 12, 2012 at 10:12

answered Dec 12, 2012 at 9:56

T.J. Crowder

1.1m201 gold badges2k silver badges2k bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Ian Over a year ago

Good comparison of change and mousemove :)

Catalin Over a year ago

Very good explanation, thank you. Saying that i will chose the second approach, how i will prevent the browser to save the top variables? I set them to null at the end of the function?

T.J. Crowder Over a year ago

@RaraituL: I've updated the answer to address your question. Best,

Catalin Over a year ago

@T.J.Crowder: Great, i was looking for it after you deleted the comment:)

T.J. Crowder Over a year ago

@RaraituL: LOL Yeah, I realized it really needed a code example, and should have been in the answer instead. :-) Glad that helped!

Collectives™ on Stack Overflow

Javascript closure variables performance

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related