FIX 11577 performance issues with large list of groups

[beerbohmdo]

Description

If you have many groups you can no longer edit a member in the admin.

Manual testing steps

As this is a performance issue, I am not sure howto create a unittest for this. But here is a task to create a massive amount of groups:

<?php

use SilverStripe\Dev\BuildTask;
use SilverStripe\ORM\DB;
use SilverStripe\Security\Group;

class GroupCreationTask extends BuildTask
{
    public function run($request): void
    {
        DB::query('TRUNCATE "Group"');
        DB::query('TRUNCATE "Permission"');

        Group::singleton()->requireDefaultRecords();

        $conn = DB::get_conn();
        $conn->transactionStart();

        $start = hrtime(true);

        for ($i = 0; $i < 3000; $i++) {
            if (($i % 100) == 0) {
                $conn->transactionEnd();
                $conn->transactionStart();

                $end = hrtime(true);
                $t = ($end - $start) / 1e9;

                printf("%d - %fs\n", $i, $t);

                $start = hrtime(true);
            }

            $parentGroup = Group::create();
            $parentGroup->Code = 'group_' . $i;
            $parentGroup->Title = 'Group ' . $i;
            $parentGroup->write();

            for ($j = 0; $j < 5; $j++) {
                $group = Group::create();
                $group->Code = 'group_' . $i . '_' . $j ;
                $group->Title = 'Group ' . $i . ' ' . $j;
                $group->ParentID = $parentGroup->ID;
                $group->write();
            }
        }

        $conn->transactionEnd();

        $end = hrtime(true);
        $t = ($end - $start) / 1e9;

        printf("%d - %fs\n", $i, $t);
    }
}

Issues

• 11577

Pull request checklist

• The target branch is correct
  • See picking the right version
• All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
  • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
• The commit messages follow our commit message guidelines
• The PR follows our contribution guidelines
• Code changes follow our coding conventions
• This change is covered with tests (or tests aren't necessary for this change)
• Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
• CI is green

[GuySartorelli]

Thanks for submitting this - I've requested a few changes below.

FYI the beta release for CMS 5.4 (which is also the feature freeze for all of CMS 5) is currently scheduled for Feb 17, so for this change to be included in CMS 5 it will need to be merged before then.
I will do my best to be quick to respond and review this PR as you update it.

[GuySartorelli]

I don't think this change should alter the searchable fields

[beerbohmdo]

Without the default search context is empty and the SearchableDropdownField does not filter.

[GuySartorelli]

We shouldn't be using that unrelated configuration here.

[beerbohmdo]

This are the default settings for SearchableDropdownField. I can create a extra config for that.

[GuySartorelli]

Why do we need this new method?

[beerbohmdo]

This is the Label for the FormField.

[GuySartorelli]

How do changes to the deduping code relate to usage of a different form field?

[beerbohmdo]

This is more or less the same issue as with the large list in the FormField. Without you are not able to save a group if you have many of them.

[beerbohmdo]

You can check the output of my generater script above with and without the change. Without the script will get slower with each iteration.

[GuySartorelli]

Like with the change in Group, we shouldn't use this unrelated configuration.

[beerbohmdo]

I created custom configs, a getter for a custom search context and added some comments.