Possible optimization for `detach` call in `_fnDraw`

[xPaw]

This code: https://github.com/DataTables/DataTablesSrc/blob/5f976cf412efc60c3c3ef521f95c80b51b7ddbf6/js/core/core.draw.js#L444

is not super efficient because it causes jquery to call removeChild on every row in the body.

Here's a hacky example that shows it could be faster:

    var newBody = $('<tbody>');
    newBody.insertBefore(body);

    body.detach();

    body = newBody;
    oSettings.nTBody = body;

As you can see it goes from 1 second to 9ms on page load on a page with 9k rows. There's certainly an opportunity to optimize this.

[AllanJard]

Its a neat idea, but I don't think it is viable unfortunately, due to it being a new element. It would mean event handlers such as:

$('#myTable tbody').on('click', 'tr', ...);

Would no longer work.

I'm astonished it takes almost a second to remove table rows - go you have a particularly large table which is being DOM loaded? I wonder if doing a native removeChild might be faster than a jQuery detach?

[xPaw]

I preload tables without using ajax yeah.

I understand making a new element isn't good here. Perhaps there's a fast way to clear out the children. Maybe .innerHTML='' or replaceChildren call.

[AllanJard]

replaceChildren is an interesting option. Are you able to provide details, or a link to your page, so I can profile it and see what difference some changes might make.

[xPaw]

https://steamdb.info/tag/492/ is my page, but any table with a lot of rows in html should do the trick.

I initially hide the table and only show it after datatable initializes as that massively helps load times.

Also I think jquerys detach uses removeChildren under the hood, it's just doing it one by one is the slow part.

[xPaw]

oSettings.nTBody.replaceChildren( ...anRows ); does work.

[AllanJard]

And how's the performance?

[xPaw]

I'm getting around 20ms for that call. But the append call after detach was similar amount of time, so it collapsed that too.

There's still some other things I'd like to take a look at, as the whole loadedInit still takes ~500ms (down from 1.39s without the detach change). 324ms of that is in _fnAddTr.

As a side note, while searching I've noticed that _pagingDraw is getting called twice because DataTable.feature.register( 'paging', ...) is getting called twice on page load when using the legacy dom option. Wouldn't consider this a big issue (I will look into migrating to layout), just something to think about.

[AllanJard]

If we switch to replaceChildren, it will need a little feature detection to make sure that is available. My goal with DataTables is to support browsers which are 10 years old and younger. That method only really became wide spread in 2020. It should also use .apply() rather than a spread operator, again for compatibility.

I'm happy to make that change, or take an updated PR as you wish.

Regarding the paging component - really good point. I've committed a fix to address that error.

[xPaw]

Compat check sounds fine.

Do you have any ideas on how to improve the speed of reading existing tr elements into internal data?

[AllanJard]

I'm afraid not. I profiled it a fair bit while doing the DT2 dev work and I think I got most of the low hanging fruit. I might have missed something of course, but I'm not sure what. If you are able to post a test case for profiling that would be useful.

[xPaw]

The link I posted above is a good test case. From my profiling, there's a few things I've noticed.

• Parsing data- attributes (like data-sort) if they're present, cellProcess is pretty expensive, partially because it does things like _fnSplitObjNotation

I have no good suggestions on how to improve this though.

• _fnFilterData is checking for presence of & in the contents, which puts it into an invisible div to then recover textContent, my question is why aren't cell contents retrieved as text to begin with in cellProcess?

[AllanJard]

Sorry, I missed the link! I'll take a look at it in the next few days.

I honestly can't recall off the top of my head why I elected to do it that way. I'd need to read over the code and comments. It might just be that I didn't think of another way, or it might have been required for the orthogonal data code paths.

[xPaw]

Here's a diff for the initially reported issue, with your suggestions:

        if ( 'replaceChildren' in Element.prototype )
        {
            Element.prototype.replaceChildren.apply( body.get(0), anRows );
        }
        else
        {
            body.children().detach();
            body.append( $(anRows) );
        }

Worth noting this speeds up every single draw call (sorting, searching, etc).

[AllanJard]

Thank you. I've committed a change to add the use of replaceChildren. All unit tests are passing with it :).

I'll close this issue for now, but if you think of any other performance improvements, just flag open a new one :)

[xPaw]

The things I've mentioned in https://github.com/DataTables/DataTablesSrc/issues/264#issuecomment-1992106903 still need some looking at.

The use of innerHTML in particular is something that needs looking at, but I don't have enough knowledge of the codebase to know if everything can be safely switched to using textContent.

[AllanJard]

Good point. I'll look into that, probably for 2.1.

[AllanJard]

I've been looking at this and the innerHTML / textContent would actually have an impact on not just DOM loaded content, but also on Ajax / JS loaded data, since it might have entities escaped and that would need to be handled. The orthogonal loading of the data might also introduce some complexities. I think if I were starting over, then I would go down that road, but at the moment, changing it would likely have a very significant impact.

Regarding the cellProcess other than perhaps a little bit of function caching I don't have much of an idea at the moment how to improve its performance. DOM interaction is always the slow part.