Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

memory leaking #177

Closed
blowsie opened this Issue Dec 12, 2012 · 17 comments

Comments

Projects
None yet
3 participants

blowsie commented Dec 12, 2012

My application is struggling with memory leakage.

To find the cause of any memory leaking I broke the whole application into parts and tested them individually.

When testing jsrender I found memory leaks when wrtiting to .html() over and over again.
I also tried very similar code in handlebars and found the memory is being hadnled much more efficiently.

jsrender

jsrender

handlebars

handlebars

My Jsrender code

<script>
        var data = {
            forms: [
                {
                    title: "Form 1",
                    subforms: [
                       {
                        title: "Subform 1",
                        groups: [
                               {
                                title: "Group1",
                                fields: [
                                    {
                                        title: "Field1",
                                        data: { name: "A", surname: "Smith" }
                                    },
                                    {
                                        title: "Field2",
                                        data: { name: "B", surname: "Smith" }
                                    }
                                ]
                               }
                        ]
                       }
                    ]

                }
            ]
        }
        $(function () {
            var repeatIsOn = false;
            window.render = function() {

                //console.time("Render");
                $("#target").html($("#formTmpl").render(data));
                //console.timeEnd("Render");

                if (repeatIsOn) {
                    setTimeout("render()", 10);
                }
            }

            $("#repeatBtn").on("click", function () {
                $(this).toggleClass('on');
                repeatIsOn = !repeatIsOn;
            })
            $("#renderBtn").on("click", render);
        });
    </script>

    <button id="repeatBtn">Repeat</button>
    <button id="renderBtn">Render</button>
    <div id="target"></div>

    <script id="formTmpl" type="text/x-jquery-tmpl">
        <ul>
            {{for forms}}
            <li>{{:title}}
            <ul>
                {{for subforms}}
                <li>{{:title}}
                <ul>
                    {{for groups}}
                    <li>{{:title}}
                    <ul>
                        {{for fields}}
                        <li>{{:title}}
                        {{:data.name}}
                        </li>
                        {{/for}}
                    </ul>
                    </li>
                    {{/for}}
                </ul>
                </li>
                {{/for}}
            </ul>
            </li>
            {{/for}}
        </ul>
    </script>

Can you suggest a way to prevent / clean up this memory leaking?

Or is there an issue with the source code?

@blowsie blowsie referenced this issue Dec 12, 2012

Closed

IE7 Performance #143

Owner

BorisMoore commented Dec 12, 2012

Thanks - nice catch. JsRender is not correctly doing the automatic caching of compiled templates when declared as script blocks. So your template is being compiled every time.

Fix coming soon, but meantime you can compile explicitly, (which is always a good practice) - following one of these patterns:

       $(function () {
           $.templates("formTemplate", "#formTmpl");
           ... 
               $("#target").html($.render.formTemplate(data));
               // or $("#target").html($.templates.formTemplate.render(data));
            ...
      });

or

       $(function () {
           var tmpl = $.templates("#formTmpl");
           ... 
              $("#target").html(tmpl.render(data));
            ...
      });

Can you verify whether this does fix the memory issue?

blowsie commented Dec 13, 2012

Hi Boris,

Thanks for the swift reply, this helps but unfortunately does not solve the issue.

I wonder if it is because all of the #data, #parent and #view type objects don't get cleaned up?

image

blowsie commented Dec 13, 2012

Update

Some good news.

You will be pleased to know I was using a slightly older version of jsrender, after updating I can see much better results.

image

blowsie commented Dec 13, 2012

How do I go about pre-compiling nested templates?

Hi blowsie,

I use nested templates a lot, and all you have to do is the exact same thing, just have an id and render it with your data... it's the same process and the first template.

blowsie commented Dec 13, 2012

I don't quite understand what you are suggesting?

So when I'm inside a template...

{{for data tmpl='how-do-i-use-precomplied-templates-here?' /}

I don't do that, I render it externally and call the data (witch normally comes from other call) and I get both the file and data externally, as I have pointed out in a stackoverflow answer

then I do something like:

var item = {
    callUrl: '/' + client + '/Winners/GetAllPossibleWinners',
    controller: 'winners',
    name: 'winnerselection',
    args: {
        prizeId: $(this).attr("data-prize-id"),
        prizeDay: $(this).attr("data-prize-day"),
        winnerId: $(this).attr("data-winner-id"),
        dayFrom: $(this).attr("data-day-from"),
        dayTo: $(this).attr("data-day-to"),
        page: 0
    },
    selector: '#tbl-winners'
};

my.data.loadTemplate(item, function (args, data) {
    // big list
    my.data.loadTemplate({
        callUrl: '/' + client + '/Winners/GetAllPossibleWinnersList',
        controller: 'winners',
        name: 'winnerselection.list',
        args: {
            prizeId: args.prizeId,
            prizeDay: args.prizeDay,
            winnerId: args.winnerId,
            dayFrom: args.dayFrom,
            dayTo: args.dayTo,
            page: 0
        },
        selector: '#winners-selection-list'
    }, function (args, data) {
        // random list
        $(".winner-selection-wrapper").find(".loading").show();
        my.data.loadTemplate({
            callUrl: '/' + client + '/Winners/GetRandomPossibleWinners',
            controller: 'winners',
            name: 'winnerselection.randomwinners',
            args: {
                prizeId: args.prizeId,
                prizeDay: args.prizeDay,
                winnerId: args.winnerId,
                dayFrom: args.dayFrom,
                dayTo: args.dayTo,
                page: 0,
                totalPages: data.possibleWinnerResult.TotalCount,
                nrWinnersToDraw: data.prize.NrWinnersToDraw
                // decimal prizeId, int? dayFrom, int? dayTo, int totalPages, int nrWinnersToDraw
            },
            selector: '#winners-selection-random'
        });

        $('html, body').animate({ scrollTop: $(document).height() }, 'slow');
        $("#winners-selection-list").hide();
    });
});

here's my current loadTemplate method

    loadTemplate = function (item, callback) {
        //my.utils.renderExtTemplate(item);

        $(item.selector).empty();
        $(item.selector).prev().find(".loading").show();

        $.ajax({
            url: item.callUrl,
            cache: false,
            data: item.args,
            success: function (data) {
                item.data = data;

                if (data.error.length > 0) {
                    my.utils.renderExtTemplate({
                        controller: 'common',
                        name: 'error',
                        data: data.error,
                        selector: item.selector
                    });
                }
                else {
                    my.utils.renderExtTemplate({
                        controller: item.controller,
                        name: item.name,
                        data: data,
                        selector: item.selector
                    });
                }

                $(item.selector).fadeIn();
            },
            error: function (xhr) {
                //this.showError(xhr);

                my.utils.renderExtTemplate({
                    controller: 'common',
                    name: 'error',
                    data: '<h4>' + xhr.statusText + '<h4><p>' + xhr.responseText + '</p>',
                    selector: item.selector
                });
            },
            complete: function () {
                $(".loading").hide();
                if (callback) callback(item.args, item.data);
            }
        });


        return false;
    },

blowsie commented Dec 13, 2012

In the code above, At what point are you actually Compiling the template?

in the my.utils.renderExtTemplate

    renderExtTemplate = function (item) {
        var file = getPath(item.controller, item.name);

        // log
        this.logMessage('loading: ' + file);

        // get and load template
        $.when($.get(file))
         .done(function (tmplData) {
             $.templates({ tmpl: tmplData });
             $(item.selector).html($.render.tmpl(item.data));

             // Tooltips
             $("[rel='tooltip']").tooltip();
         })
        .fail(function (result) {
            //this.logMessage('Loading template has failed: ' + result.statusText);
            alert('Loading template has failed: ' + result.statusText);
        });
    }

blowsie commented Dec 13, 2012

Thank you for sharing your code.

To me, it seems like alot of function calls to actually render something?
Also with this approach you have to write both JavaScript and the template for every template you create.

Would it not be an idea to cache the compliled template in your my.utils.renderExtTemplate?


This approach in my templates would lead to an insane amount of code and it would be very difficult to manage.

I think id rather use the following kind of approach
{{for data tmpl='~childTemplate('#tmplName')' /}

And then create the helper to both compile and cache the template.

This approach infact is similar to this issue i just found answered by Boris
#165

this are global methods, and I do render a lot, so I simple call my loadTemplate and it will get an external file, plus call the API and get all information to be bind into the external template.

for something like this:

{{for data tmpl='~childTemplate('#tmplName')' /}

you need to pass the data in the first call as well, and I do not want that, as each template get's their own data from an API call.

You will learn all this in John Papas Puralsight Videos

http://pluralsight.com/training/courses/TableOfContents?courseName=jsrender-fundamentals

You can get free trial, and if you're a member of Website Spark you will get 90 days trial

Owner

BorisMoore commented Dec 13, 2012

Going back to blowsie's original question, "How do I go about pre-compiling nested templates?" there are several examples of different ways of naming/compiling/accessing templates here http://borismoore.github.com/jsrender/demos/variants/variants.html, including as nested templates.

The latest version of JsRender has an additional feature that allows you to compile a template as a resource 'owned' by another template `$.templates(..., parentTemplate). See many permutations of this (and other template resources) in this test page http://borismoore.github.com/jsrender/test/test-pages/nested-template-resources.html.

BorisMoore added a commit that referenced this issue Dec 14, 2012

Owner

BorisMoore commented Dec 14, 2012

This has been fixed in commit 24. Can you verify whether this corrects the memory leak you were seeing? Thanks!

Owner

BorisMoore commented Dec 14, 2012

Closing. Please reopen if you still see the issue... Thanks...

@BorisMoore BorisMoore closed this Dec 14, 2012

blowsie commented Dec 20, 2012

I had resolved this myself by writing my own helper.

getTmpl: function (tmpl){
        if (!tmplCache[tmpl]) {
            tmplCache[tmpl] = $.templates(tmpl);
        }
        return tmplCache[tmpl];
    },

Looking at your changes, I should no longer require this?

Ps. I cant re-open issues :P

Owner

BorisMoore commented Dec 20, 2012

With the changes, compiled templates accessed as selectors pointing to script elements don't need to be cached by your helper, since they are now correctly cached along with the script element. Your helper is also caching templates which you pass as a string. That part is not done by JsRender, so depends whether that is a scenario for you. The 'best practice' for JsRender with compiling from strings is either to compile by name, or to hold on to the compiled template objects, and call the render method on that.

Didn't realize the 'creator' of an issue couldn't reopen it! But in response to a request to reopen, I will do so :),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment