fix: Form performance improvements

[bastianallgeier]

Chunks:

• refact: New ::emptyValue and ::fillWithEmptyValue methods for Field and FieldClass #7663
• refact: Cache field options #7664
• refact: Cache block forms for each fieldset #7665

Description

After looking closer at this issue and @afbora research #7641 I found multiple ways how we can improve/fix this. We have to decide if we do all the steps at once, or if we want to break them down, but I'm using this PR as a big sweep first to give you the bigger picture.

Benchmark results

To not step in the dark with premature improvements, I've created a little benchmark setup for the sandbox with a new CLI command that can help us test our changes more easily.

The results of this PR are impressive. I'm benchmarking two scenarios.

Form::reset()

To reset all form fields took forever before the PR because the old Field class needed to rerun all prop and computed prop methods to make sure that there's not an invalid state for one of them which depends on the value.

I'm running this 10 times in a loop.

foreach (page('fields')->children() as $fieldPage) {
  Form::for($fieldPage)->reset();
}

Before

Bench: Time: 00:11.542, Memory: 24.00 MB

After (2 times faster)

Bench: Time: 00:05.704, Memory: 24.00 MB

$page->panel()->view()

Creating all the props for the view needs to create two forms: one for the latest version and one for the changed version (if it exists) This makes the reset problem from above even worse because we need to reset the form when we want to reuse it for two versions and this could also add up even more if there are nested forms (structures, blocks, etc.)

Again, I'm running this 10 times in a loop.

foreach (page('fields')->children() as $fieldPage) {
  $fieldPage->panel()->view();
}

Before

Bench: Time: 00:16.887, Memory: 24.00 MB

After (3,2 times faster)

Bench: Time: 00:05.832, Memory: 24.00 MB

This clearly shows how the problem escalates with more form setup and reset tasks.

This should have positive performance side-effects to pretty much any part of the panel.

Running benchmarks

To test the differences, you can use the Kirby CLI in the sandbox root directory. Make sure to pull the latest sandbox changes. There's a new way to set the sandbox URL with a new custom sandbox config. In the root directory, you'll find a new sandbox.config.sample.php. Rename it to sandbox.config.php and add your custom URL in there. All options in the file will be merged with the Kirby config of the sandbox. The sandbox.config.php is automatically ignored.

Once this is done, you can run

kirby sandbox:bench:fields

… and …

kirby sandbox:bench:views

Changelog

✨ Enhancements

• Cache options in the options mixin for fields.
• Cache block forms by type in the blocks field.
• Cache columns in the structure field
• Cache color options in the color field
• Cleaned up object field with the new form methods and caching, similar to the structure field.
• Improve type hinting in the options mixin for fields
• New ::fillWithEmptyValue method for Field and FieldClass. This is used in Fields::reset to reset the value to a correct empty one. The naming is not great, but we cannot use reset, as it exists as option in the toggles field and would collide. It's at least clear enough. This will make sure that resetting a field will be as fast as possible without reevaluating props and computed props in the old rusty Field component. This is one big source for performance hits.
• New Field::handlerExists and Field::handlerCall methods. Those are not directly related to this PR, but simply help to remove some redundant code. We can totally do this in a separate PR.

♻️ Refactored

• Use Form instead of the Fields class in Kirby\Panel\Model
• Remove an unused class declaration in the Form class

For review team

• Add lab and/or sandbox examples (wherever helpful)
• Add changes & docs to release notes draft in Notion

[distantnative]

@bastianallgeier I am having a bit of a hard time following all the different changes in this PR, so apologies if I haven't quite understood all the aspects. But if I look at the changes necessary in all the different options fields, isn't this then a breaking change for all plugin fields using the options mixin etc.? Or custom fields now would need to define an emptyValue?

[bastianallgeier]

It's quite much. I can really break it down more. But I kind of also wanted to use this PR to verify that the performance gains are actually there and as big as in my tests.

The fields don't have to implement the emptyValue method. It's a performance enhancement that is helping to make things faster, but not necessary for custom fields. When your custom field does not use a lot of computed props, you won't see major performance benefits anyway. The worst offenders are fields that compute heavy stuff like the options.

You also have to keep in mind that this issue is only related to the Field class. Anything that's built on top of the FieldClass class, does not have those performance issues. This is where we are heading anyway.

[distantnative]

Suggestion: I do understand that we need to add emptyValue as attr to the array-notation fields, what feels a bit like a hack but probably no better way for these. Though here in the class-based fields, I don't know... doesn't come across as a super satisfying way of doing this. Also, if we look ahead to named-parameter constructors, wouldn't it be better/cleaner to define the empty value via the default value of the value property?

[bastianallgeier]

We could only handle this with a reflection if we want to get default value of the value prop. I'm not sure if that is worth it to be honest.

[distantnative]

@bastianallgeier But if a custom field uses the options mixin and has a options prop defined, it will not reset the optionsCache as this PR does for the core fields (and thus seems required for a field to properly work with these changes). No?