Speed up Download Spreadsheet from SearchKit results

[larssandergreen]

Overview

Downloading a spreadsheet from SearchKit results is often slow when you have even a thousand rows. Timeouts are common, depending on your environment, with a few thousand rows.

It turns out that almost all the execution time was being used to generate the menu column, which of course isn't included in the spreadsheet download, so doesn't need to be generated. I suspect there is also some work to be done to make that faster in general to speed up results in SearchKit (not a big deal with 50 rows, but if you increase the page size it gets slower).

Related issue.

Before

Slow! Timeouts! Can't download more than a couple thousand rows.

After

At least an order of magnitude faster, for simple rows. Timeouts much less likely.
Can download 10,000 plus rows (I was hitting memory limits while testing, rather than timeouts).

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[larssandergreen]

Related issue on the potential to speed up SearchKit in general by optimizing how we check access for the menu link (View / Edit / Delete), which is where a lot of the time is going.

[colemanw]

I like the concept! 2 thoughts about the implementation:

1. Unsetting array items within a foreach loop is not stable because the unset reindexes the items which haven't been iterated yet. Easiest workaround is to foreach(array_reverse(....
2. I think the conditional checking that type is ['menu', 'buttons', 'links'] is a bit too specific. A more generic check would be if the column has a key. I think the following code could be removed from line 78 and that condition used in your new loop instead:

     if (!empty($col['key'])) {
       $columns[$index] = $col;
     }
   

[larssandergreen]

I don't think unset reindexes an array in PHP (at least not since PHP 7), so that should be OK.

Yes, that makes more sense to check for the key, have updated.

[colemanw]

Ok, great @larssandergreen please just add a comment on l57 to explain that non-spreadsheet columns are being unset for performance reasons.

[larssandergreen]

@colemanw thanks, comment added

[colemanw]

Thanks @larssandergreen for this PR, I'm sure everyone with big spreadsheets to download will appreciate it!

[colemanw]

@larssandergreen I'm curious to know what tool you used to determine which part of the code was taking the most time.