Performance: Shall I use more or less temp variables?

I wonder what would be the better way to deals with all the ->xx() functions.

Would it be better to use all inline

$visibleEntries = $entries->filterBy('visibility', 'visible')->merge($entries->filterBy('visibility', 'auto')->limit($g->maxentries()->int()))->sortBy('sort', 'desc');

or use temporary variables

$n = $g->maxentries()->int();
$visibleEntries = $entries->filterBy('visibility', 'visible');
$visibleAutoEntries = $autoEntries->limit($n);
$visibleEntries = $visibleEntries->merge($visibleAutoEntries)->sortBy('sort', 'desc');

? The latter is much more readable but what about the performance?


1 Like

That mostly depends on whether you need the intermediary values later on again. If you do, store the results in a temporary variable, otherwise you don’t need to.
If you use more variables than you need, the performance will be the same, but your application will need more RAM because it needs to keep everything in memory and can’t clean it up.

Regarding readability: What about this?

$visibleEntries = $entries->filterBy('visibility', 'visible')
                          ->merge($entries->filterBy('visibility', 'auto')
                          ->sortBy('sort', 'desc');
1 Like

Thanks :slight_smile:

For my code I only need $visibleEntries so using your suggestion for the readable oneliner is a nice idea. I didn’t know that I can spit thins up like this.

Storing the result of $g->maxentries()->int(); in a variable has no performance impact, it’s just an integer in a variable. The actual cost is querying a metadata key (or maybe that cost was already paid when you got the page object represented by $g, I’m not sure when Kirby reads from the filesystem and what it keeps in memory exactly.

Also I’m not sure if it would have better performance than using $pages->merge(), but if the logic “an entry is visible if it has a visibility field whose value is either visible or auto” is something you want to reuse, you could declare it as a function in a plugin, and ->filter() on it.

function entryIsVisible($page) {
  return in_array($page->visibility()->value(), ['visible', 'auto']);

$visibleEntries = $entries->filter('entryIsVisible')
                          ->sortBy('sort', 'desc');

Also maybe you want to sort before you limit? There’s a performance penalty for sorting on many entries but if you limit first, you’re going to sort on the wrong data. If $entries already contains correctly ordered data but you want it in the reverse order, you could use ->flip() instead of ->sort(), it should be quicker.

Thanks. But an entry can have one of four states and depending on that they are either visible in one or another list (or not at all). And the limit only applies to the auto entries so if $n = 3 and there are two entries with visible the list will have five items. Therefore I can’t use the visible-OR-auto logic …

@fvsch: Instead of creating a function in a plugin, you could use a filter with a callback for this use case.

That’s what I’m doing in the above example, except I declared the function before referencing it, which is more readable in my book. (A bit less callback-hellish.)

I was mentioning a plugin because you could declare the entryIsVisible function in site/plugins/myplugin.php and it will usable in all templates, which is useful if you need to use that filtering logic in several places.

Nope … I need it only in one template and can’T even use a control because it’s a one page (I could use the controls of that one-pager template of course but I like to have it in the snippet for the according section).