@tsteur opened this Pull Request on October 23rd 2019 Member

refs https://github.com/matomo-org/matomo/pull/15037

Currently it is checking if a data table is empty when means twig will call the toString method of our dataTables

image

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.

@diosmosis commented on November 4th 2019 Member

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).

@tsteur commented on November 4th 2019 Member

@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.

@diosmosis commented on November 4th 2019 Member

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 'ROWS: $rowCount' or '' if no rows and see if the tests pass.

This Pull Request was closed on November 4th 2019
Powered by GitHub Issue Mirror