Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rendering DropdownField can be really slow for many instances #10174

Open
lekoala opened this issue Dec 10, 2021 · 5 comments
Open

Rendering DropdownField can be really slow for many instances #10174

lekoala opened this issue Dec 10, 2021 · 5 comments

Comments

@lekoala
Copy link
Contributor

lekoala commented Dec 10, 2021

Affected Version

4.x

Description

I have a project with an editable gridfield with many dropdowns (something like 50 rows and 2 dropdowns per row) and it's super slow to render. Here is a screenshot given by the DebugBar in the admin. It takes about 9 seconds to render all the dropdowns (it's faster on production, but still quite slow).

Screenshot 2021-12-10 135247

(the CMSTabSet is also super slow, but that's the next issue i'll be handling :) )

In the meantime, i created this
https://github.com/lekoala/silverstripe-base/blob/master/src/Forms/FastDropdownField.php
which is not really a solution, but at least it works for my use case and make the page much faster where i can prerender the template and avoid calling the view engine each time which seem to be the culprit, because it takes less than 1s now.. should be also be there are also a couple of other dropdowns in other tabs).

Steps to Reproduce

Create an editable gridfield that is not paginated with many dropdowns. Here is some sample code below, simplified for the purposes of this ticket (not tested or isolated on a fresh project).

                $c= GridFieldConfig_RecordEditor::create();
                $c->addComponent(new GridFieldTitleHeader());
                $c->removeComponentsByType(GridFieldSortableHeader::class);
                $c->removeComponentsByType(GridFieldPaginator::class);
                $c->removeComponentsByType(GridFieldPageCount::class);
                $c->removeComponentsByType(GridFieldDeleteAction::class);
                $c->removeComponentsByType(GridFieldAddNewButton::class);
                $gf= new GridField('Records', 'Records', $this->owner->List(), $c);
                $fields->addFieldsToTab('Root.Records', $Records);
        
                $c->removeComponentsByType(GridFieldDataColumns::class);
                $c->addComponent($cols = new GridFieldEditableColumns());
               
                $dataMap= $this->dataMap();
                $fields = [];

                $fields['RelationID'] = [
                    'title' => 'Relation',
                    'callback' => function ($r, $name, $grid) use ($dataMap) {
                        $dd = new DropdownField($name, null, $dataMap, $r->$name);
                        return $dd;
                    }
                ];
                $fields['OtherRelationID'] = [
                    'title' => 'Other Relation',
                    'callback' => function ($r, $name, $grid) use ($dataMap) {
                        $dd = new DropdownField($name, null, $dataMap, $r->$name);
                        return $dd;
                    }
                ];
                $cols->setDisplayFields($fields);

Maybe worth exploring

Simplifying the rendering by not using the view engine to render the option list. I'm going to explore that as well (would probably be cleaner than what i did) and update this issue if I found anything

@lekoala
Copy link
Contributor Author

lekoala commented Dec 10, 2021

So indeed it seems that using the template slow things down quite a bit and using plain php code is faster at least from what i can tell (it's not exactly easy to compare from within a complex project)

I think it's probably worth dropping the template that is not adding much value nevertheless but maybe more testing needs to be done.

I've updated
https://github.com/lekoala/silverstripe-base/blob/master/src/Forms/FastDropdownField.php
And it's working fine as far as i can tell in my project

@sb-relaxt-at
Copy link
Contributor

I had some similar experiences (related to other FormFields) that using a lot of template calls causes serious performance problems.

@lekoala
Copy link
Contributor Author

lekoala commented Jun 26, 2023

small update on this because this is becoming really annoying :-)

with a couple of dropdowns with a country list

image

without the dropdown (just went to the cache folder and removed the content on the .dropdown.ss cache file)

image

yes, 3s different for a couple of dropdowns. yes, it's slow due to xdebug profiling the whole thing, but still, i didn't expect such a large difference. i'm going to try to pinpoint where this is coming from.

@sb-relaxt-at
Copy link
Contributor

Agreed, we have a component for providing some kind of inline editing by using form fields for editing nested entities. When switching some hidden fields (used for storing internal information like sort order) from templates to plain html strings it resulted in a significant performance improvement.

From my experience having xdebug enabled can have an extreme performance impact in Silverstripe projects. You might want to try it without, I assume the impact in a production environment will be a lot smaller. Nevertheless I would welcome any performance improvement to the template layer :-)

@lekoala
Copy link
Contributor Author

lekoala commented Jun 26, 2023

it seems that every call to $scope->locally() is really slow
which is kind of expected since it does $this->localStack[] = array_splice($this->itemStack, $this->localIndex + 1); under the hood. so there is a whole lot of array copying happening.

i would qualify this as a design flaw from the template itself. it's quite visible for a dropdown with lots of options, but it's basically affecting all form fields

the long term solution would be:

  • fixing the template language itself (i don't see how that could be easily done)
  • use less templates, and avoid templates (or at least, looped over templates) for forms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants