Skip to content
This repository was archived by the owner on Feb 3, 2022. It is now read-only.

Performance Improvement#128

Closed
sboisvert wants to merge 2 commits intocds-snc:k8sfrom
sboisvert:patch-1
Closed

Performance Improvement#128
sboisvert wants to merge 2 commits intocds-snc:k8sfrom
sboisvert:patch-1

Conversation

@sboisvert
Copy link
Copy Markdown

Add deferRender = true to improve the performance in older browsers by only creating the DOM nodes for the items that are currently being displayed on page 1.

See https://datatables.net/reference/option/deferRender

maxneuvians and others added 2 commits April 8, 2019 11:19
* Modification for deploying on k8s

* Small fix on dockerfile

* Added CI workflow file

* Ignore pip pinning in CI
add deferRender = true to improve the performance in older browsers by only creating the DOM nodes for the items that are currently being displayed on page 1.

See https://datatables.net/manual/ajax
Copy link
Copy Markdown
Member

@timarney timarney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great. For this PR thinking we should only include the patch for the single file change in tables.js

Copy link
Copy Markdown
Contributor

@obrien-j obrien-j left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebase to get rid of already merged commit plz

Can you also profile this on a datastore of say 2000 records, to get an idea of improvement in load time plz.

@timarney
Copy link
Copy Markdown
Member

timarney commented Apr 8, 2019

Tested with a sandbox database 3314 records.

Using BrowserStack + Sandbox DB to localhost so not super accurate but seems like for IE 11 this speeds things up by guessing around 8secs.

The big thing in this case is it's not creating a bunch of DOM nodes that it doesn't need off the top.

As an example to help illustrate this, if you load a data set with 10,000 rows, but a paging display length of only 10 records, rather than create all 10,000 rows, when deferred rendering is enabled, DataTables will create only 10.

@obrien-j
Copy link
Copy Markdown
Contributor

obrien-j commented Apr 8, 2019

@ptd-tbs @sayaHub We might want to cherry pick this into existing production deployment.

@timarney
Copy link
Copy Markdown
Member

timarney commented Apr 8, 2019

@sboisvert

The K8s branch was merged #127 so you can rebase to master then make you one line update.

if (!options.deferRender) options.deferRender = true;

@timarney timarney mentioned this pull request Apr 9, 2019
@timarney
Copy link
Copy Markdown
Member

timarney commented Apr 9, 2019

Closing in favour of #129

-> same fix

@timarney timarney closed this Apr 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants