New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Further dataTable performance improvements #7502
Conversation
$values[$key] = array($this->getColumnValue($row), $row->getColumn('label')); | ||
} | ||
} else { | ||
foreach ($rows as $key => $row) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only numberSort
actually uses a secondary column (label
). For all other sort callbacks we do not need the secondary column so we can simply pass the value directly which will make sorting by label faster.
// we do not use getRows() as this method might get called 100k times when aggregating many datatables and | ||
// this takes a lot of time. | ||
$row = $tableToSum->getRowFromId(DataTable::ID_SUMMARY_ROW); | ||
if ($row) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not strictly related to this change, maybe it would increase code clarity if DataTable::$summaryRow were stored in DataTable::$rows? I haven't looked to see if this would be do-able or not, maybe something to think about in another pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could point, I think that would simplify many things and even improve performance sometimes. I might have a look at this end of week or next week. Not sure if I will find time though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick note: It might be not as easy. Eg getFirstRow
will be more complicated as it should not return the SummaryRow as a first row if there are other rows. Also getLastRow
should return the summary row first. Those kinda implementations will get a bit more complicated.
Also sort etc. might get a bit harder since we shouldn't sort the summary row. Here he would have to create a new $rows
array without the summary row.
👍 |
Further dataTable performance improvements
Sorry for having another pull request for this. I noticed those issues while profiling an archiving run that takes 50 minutes and while I was looking into some other things.
This PR will improve performance when using DataTables.