Use of writeComponents via GridFieldEditableColumns causing performance issues

[MrJamesEllis]

Hi folks

With a form with File Upload Fields I'm seeing recursive write requests starting from the selected upload folder into its children (folders and files). For a form with a not-small child file system under the upload folder, adding a field / saving a form at the least results in a wait while the writeComponents does it thing.
Seeing this with elemental userforms module, but I don't think the module is to blame. Setting writeComponents to false, not true, at https://github.com/symbiote/silverstripe-gridfieldextensions/blob/4.0.0/src/GridFieldEditableColumns.php#L169 speeds things up.

This could be related to the discussion at #733 and the various issues linked to that.

Steps

1. Create a form
2. Add File Upload Fields, select a upload folder
3. Dump files in the upload folder
4. Save the form
5. Add a new field (of any type), save the form
6. Observe save result/time

Actual Results

Whenever the form is saved, the following happens:

1. GridFieldEditableColumns::handleSave() is called
2. For each item found, write is called with writeComponents param = true
3. The folder linked to the File Upload Field is written, results in a onAfterSkippedWrite
4. Folder::updateChildFilesystem is called
5. Each child write() is called (results in skipped writes)
6. Each child is validated via validate()
7. The form eventually saves, times out or fails validation on one of the fields/field relations

Expected results

When a form is saved, only the changed fields are saved and their components are not saved.

1. Add a new field, save, only that field is saved.
2. Change a field type, title, only that field is saved.
3. Reorder field, only changed fields are saved

The fix I think would be to only save the form and its changed fields, and not the field components. I don't know if that is going to cause any regressions. Someone may have tried that already but I can't see the point of writing/validating possibly hundreds of unchanged linked records when a form is saved.

Because each child has validate() called, any extension that might perform validation / onbeforewrite is hit. This can result in further issues / time spent depending on what the extension is doing.

I thought subclassing GridFieldEditableColumns and using that in the userforms module, but without writeComponents could be a way forward? It still saves every single field but that's better than current.

Thanks
James

[NightJar]

I see you've found symbiote/silverstripe-gridfieldextensions#350

🤔 I didn't get the PR merged yet because of variable naming I haven't found time to re-address. I do still intend to.
That change coupled with this DataExtension (below) is what gets applied to UserDefinedForm in my project in order to circumvent the issue you're having.

Once the GridFieldExtensions module PR is merged the intention is to then also make this more permenant in the UserForms module, too.

use SilverStripe\Forms\FieldList;
use SilverStripe\ORM\DataExtension;
use Symbiote\GridFieldExtensions\GridFieldEditableColumns;

class UserFormDontWriteOnHandleSave extends DataExtension
{
    public function updateCMSFields(FieldList $fields)
    {
        $fields->fieldByName('Root.FormFields.Fields')
            ->getConfig()
            ->getComponentByType(GridFieldEditableColumns::class)
            ->setSkipWriteComponents([
                'recursive' => true,
                'skip' => [get_class($this->owner) => [$this->owner->ID]],
            ]);
    }
}

[MrJamesEllis]

Thanks for the workaround.

The default of writing components in GridFieldEditableColumns causes the unexpected behaviour of writing/validating every file in the designated upload folder whenever the File Upload Field is saved in the grid field.

If we had a custom editable form field in a userform that linked to a Page record, would the Page record be written on every field save? Probably something that we wouldn't want to happen automatically.

Maybe a starting point is to find out if components of the editable fields need to be recursively written in the userform editable fields interface, or if it's just a result of the default in GridFieldEditableColumns ?

I don't think GridFieldEditableColumns should be writing components at all by default, feels like something a developer should turn on case-by-case basis. Changing it will cause BC issues, tho :(