Many tables on a single page - performance issue

Many tables on a single page - performance issue

krzychokrzycho Posts: 7Questions: 0Answers: 0
edited March 2013 in Bug reports
Hi All,

On my page I render multiple tables. DataTables are optimized for many rows, but in a single table. In case of having multiple tables on a single page (e.g. 13) it can be quite slow I found two performance issue on IE 8/9 that could be easily fixed.

I.
Function [code]_fnBrowserDetect[/code] can be very slow on IE 8/9. It does not cache the result, so if you have e.g. 13 tables on a single page it will be called 13 times and it can be huge performance issue. We could cash a result of this function somewhere.

II.
[code]
var oSettings = $.extend( true, {}, DataTable.models.oSettings, {
"nTable": this,
"oApi": _that.oApi,
"oInit": oInit,
"sDestroyWidth": $(this).width(),
"sInstance": sId,
"sTableId": sId
} );
[/code]

Property sDestroyWidth is used only when bAutoWidth is true and only in the fnDestroy function.

but calculation of its default value is done always

[code]"sDestroyWidth": $(this).width(),[/code]

On my page on IE 8/9 it takes some significant amount of time. I think it could be calculated only if it's needed.

Replies

  • allanallan Posts: 63,552Questions: 1Answers: 10,477 Site admin
    I. Excellent point. I will add that.

    II. The problem is, how do you know if it is needed? You only know that if the table is destroyed and DataTables can't know up front if you are going to destroy the table at some point in future.

    Allan
  • krzychokrzycho Posts: 7Questions: 0Answers: 0
    Hi Allan,

    Regarding calculating table width. Wouldn't be enough if you use css('width') instead of width()?

    Correct me if I am wrong. You want to restore original width of the table, because you change it during initialization by using css('width')? If yes it should be enough to check if table had width set directly on the table and if yes you could restore it if not in destroy method you could just remove it.

    $(this).get(0).style.width can give you width set directly on the element. Just in case you could also store width set in the width table attribute

    This would make that destroy method would really remove all modifications done by the plugin.

    Currently if you just create a table and then destroy it you will have extra width style set, won't you?
  • allanallan Posts: 63,552Questions: 1Answers: 10,477 Site admin
    My understanding of `css('width')` is that is uses the getComputedStyle result for the table - not the stylesheet value (which is why it reports 800px rather than, what would be a much more useful 100%). So what would the difference be?

    > Currently if you just create a table and then destroy it you will have extra width style set, won't you?

    Correct - but I don't see a way around that at the moment, since css('width') will report a width, regardless of if there is actually a width assigned in the CSS, since it is using the computed style.

    Allan
  • krzychokrzycho Posts: 7Questions: 0Answers: 0
    Sorry, I ment [code] $(this).get(0).style.width[/code] instead of [code] css('width')[/code]
  • allanallan Posts: 63,552Questions: 1Answers: 10,477 Site admin
    That only works when the width has already been set by Javascript though: http://live.datatables.net/igoceb/edit#javascript,html . The whole point of that line is to capture the width of the table before DataTables' Javascript messes around with the width of the table.

    Allan
  • krzychokrzycho Posts: 7Questions: 0Answers: 0
    I am not sure if I understood. Your example shows that it should work.

    I see following steps:
    1. Before initialization store original width set directly on the element
    1.1 Store, if exists, width from inline style of the table using:
    [code]var originalInlineWidth = $(this).get(0).style.width[/code]
    1.2 Store, if exists, width from table width attribute using:
    [code]var originalWidthAttrValue = $(this).prop('width')[/code]

    2. Create DataTables

    3. Destroy DataTables
    3.1 Restore original inline style, if existed before, using
    [code]$(this).get(0).style.width = originalInlineWidth[/code]
    3.2 Restore original table width attribute value, if existed before, using:
    [code]$(this).prop('width', originalWidthAttrValue)[/code]
  • krzychokrzycho Posts: 7Questions: 0Answers: 0
    I've created a small example: http://jsfiddle.net/NZsHT/
  • allanallan Posts: 63,552Questions: 1Answers: 10,477 Site admin
    That works because you have `style="width..."` . If you do it in CSS, then it doesn't work: http://jsfiddle.net/NZsHT/1/

    Allan
  • krzychokrzycho Posts: 7Questions: 0Answers: 0
    edited April 2013
    This is what is expected. If the width was set using style and you destroy the table then width from the style will be applied again.

    Correct me if I am wrong, fnDestroy should revert all changes done by DataTables, it means that after calling this method table should look like it did before calling dataTables constructor.

    You keep original width in sDestroyWidth, because you set width on table element. It means that if a table did not have inline style before dataTables it should not have it after dataTables are destroy as well.

    See following example: http://jsfiddle.net/NZsHT/3/
  • allanallan Posts: 63,552Questions: 1Answers: 10,477 Site admin
    That does look good to me. You are correct in how fnDestroy should work, I just wasn't sure how to achieve it, but this looks like a good method. Let me please with it a bit more and I will commit it in.

    Thanks for looking into this.

    Allan
  • allanallan Posts: 63,552Questions: 1Answers: 10,477 Site admin
    First issue (scrollbar width calculation) is resolved in this commit: https://github.com/DataTables/DataTables/commit/ad0e085 . Will be in the 1.10 release :-).
  • allanallan Posts: 63,552Questions: 1Answers: 10,477 Site admin
    And second issue (width restoration) is resolved in this commit: https://github.com/DataTables/DataTables/commit/b1d88c1 . It also will be in the 1.10 release, or use the tip of DataTables dev from the git repo :-).

    Thanks again for these suggestions and the discussion on them!

    Regards,
    Allan
  • krzychokrzycho Posts: 7Questions: 0Answers: 0
    That's great! Thanks!
This discussion has been closed.