Improve serialize dataTable performance #12289
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This seems to improve the performance of serializing a data table which is useful when archiving. In my case it made the archiving of a monthly archive twice as fast from about 120 seconds down to like 70seconds.
tsteur
added
c: Performance
It is known that array_key_exists is quite a bit slower than `isset`. As this is executed easily a few million times this makes a difference in performance but here `array_key_exists` and `isset` work the same in logic as we never set `$consecutiveSubtableIds[$id]=null`
tsteur
commented
Nov 20, 2017
| @@ -1292,7 +1293,7 @@ public function getSerialized($maximumRowsInDataTable = null, | |||
| // we then serialize the rows and store them in the serialized dataTable | |||
| $rows = array(); | |||
| foreach ($this->rows as $id => $row) { | |||
| if (array_key_exists($id, $consecutiveSubtableIds)) { | |||
| if (isset($consecutiveSubtableIds[$id])) { | |||
There was a problem hiding this comment.
It is known that array_key_exists is quite a bit slower than isset. As this is executed easily a few million times this makes a difference in performance but here array_key_exists and isset work the same in logic as we never set $consecutiveSubtableIds[$id]=null
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This seems to improve the performance of serializing a data table which is useful when archiving. In my case it made the archiving of a monthly archive almost twice as fast from about 120 seconds down to like 70seconds.
Before:
After
Why?
getSerializedis called here 385 times and took before 51 seconds, afterwards about 10 seconds. The thing is thatgetSerializedgets called recursive, and on the 2nd layer, it is called almost 8000 times, and on the third layer (when having 3 nested dataTables) it is called 40.000 times:When it is called 40.000 times, it doesn't take much time but on the 2nd layer, it spends about 40seconds in the method itself (excluding calling any methods). Looking at the function itself I realized the
$aSerializedDataTable + $subTable->getSerialized($maximumRowsInSubDataTable, $maximumRowsInSubDataTable, $columnToSortByBeforeTruncation, $aSerializedDataTable)call which merges BIG arrays over time and seems to be the slow part.So I changed it to
getSerialized($maximumRowsInDataTable = null, $maximumRowsInSubDataTable = null, $maximumRowsInSubDataTable = null, $columnToSortByBeforeTruncation = null) $columnToSortByBeforeTruncation = null, $aSerializedDataTable = array())which made it like 2 or 3 times slower because it is copying the arrays even more etc.But when passing as reference it is suddenly fast (less copying):
It is down from 40 seconds to 2 seconds.