Slight performance improvement when rendering data table visualisations #15044
Conversation
tsteur
added
c: Performance
| @@ -2,7 +2,7 @@ | |||
| {% include visualizationTemplate %} | |||
| {%- else -%} | |||
|
|
|||
| {% set isDataTableEmpty = (dataTable is empty or dataTableHasNoData|default(false)) %} | |||
| {% set isDataTableEmpty = (dataTable is not defined or dataTableHasNoData|default(false)) %} | |||
There was a problem hiding this comment.
might it be possible that dataTable is null in some cases? If so it might be better to maybe also check for dataTable is null
|
I just merged this, but it seems like it would be easy for this to happen again. I wonder if we should just remove the __toString() method from DataTable (in matomo 4, I suppose). |
|
@diosmosis just not sure why it was added in the first place? If it's not needed for something else then could just change it to return a simple string or so... it should otherwise not make performance much slower if it happens again. If we know the tostring is not needed though then be good. |
|
It would probably be difficult to see everywhere a datatable could be converted into a string... I guess we could just change it to return |
refs #15037
Currently it is checking if a data table is empty when means twig will call the toString method of our dataTables
which means for regular data tables it will use the HTML renderer just to check if the table is empty, for Maps it will use the console renderer. They always return a non empty string though meaning the check is kind of useless and only consumes CPU.