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

Custom fields being checked even when not displayed, performance impact #17889

Closed
bjxrn opened this issue Sep 6, 2017 · 14 comments
Closed

Custom fields being checked even when not displayed, performance impact #17889

bjxrn opened this issue Sep 6, 2017 · 14 comments

Comments

@bjxrn
Copy link

bjxrn commented Sep 6, 2017

Steps to reproduce the issue

Create a content Category
Set 10 custom fields of the Media type for that category, make sure the fields are set to be visible on administration only.
Make 10 articles for that category. Don't enter any content. Don't set any values for the custom fields.
Make a category blog menu item for that category.
Visit the category blog.

Expected result

The category blog page loads without any performance degradation because the media fields aren't visible on the front-end/site, only in the admin.

Actual result

The category blog page loads a lot slower.
Joomla goes through the fields for each article, and for media types checks whether the value for the field is a file and if the file exists, which takes time.

System information (as much as possible)

FreeBSD 11
Apache 2.4
PHP FPM
PHP 7.0
Joomla 3.7.5

Additional comments

We were hoping to use the custom fields for a site to set various extra fields, but using the Media fields seriously degrades performance. I was hoping turning the Site display off would help things, but it looks like it has no impact.

I think it would be better if Joomla didn't try to validate Media fields unless they were set to being displayed.

@bjxrn
Copy link
Author

bjxrn commented Sep 6, 2017

Actually, doing some more testing. It looks like fields in general slow down rendering, not just media fields.

Doing the above test again but with text fields also resulted in a significant slowdown.
Having the blog without any custom fields resulted in the document loading in ~300ms.
Adding 10 custom fields (text) meant the page loaded in around 10 seconds, so about 30 times slower.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17889.

@ghost
Copy link

ghost commented Sep 6, 2017

@laoneo can you please have a Look?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17889.

@ggppdk
Copy link
Contributor

ggppdk commented Sep 6, 2017

10 text fields x 10 articles x 4 events = 400 field displays prepared

the above include

10 text fields x 10 articles x 4 events x 11 field types x 1 JFolder::files() = 4,400 JFolder::files()
10 text fields x 10 articles x 4 events x 11 field types x 2 file_exists = 8,800 file_exists()

@ggppdk
Copy link
Contributor

ggppdk commented Sep 6, 2017

At least the file exists can be almost zeroed with no B/C break

@ggppdk
Copy link
Contributor

ggppdk commented Sep 6, 2017

I have made a PR to remove the 4,400 JFolder::files() and 8,800 file_exists()
done in a category view for 10 text fields and with 10 articles

about the 400 field displays prepared
-- arguably a field can have a layout that display something even if not having a value !!

so we need to prepare field's display always ! right ?

but at least every field is specifying the event that should be used to display it
so this
10 text fields x 10 articles x 4 events = 400 field displays prepared

can be made ??
10 text fields x 10 articles x 1 events = 100 field displays prepared

@ggppdk
Copy link
Contributor

ggppdk commented Sep 6, 2017

@bjxrn

if you make fields be of type 10 textarea / editor fields instead of "text",
-- and you also add them a value

then the 10 seconds can become 40 seconds because of content plugin triggering (that include slow plugins like emailcloak)

10 text fields x 10 articles x 4 events = 400 times field display created = 400 content plugin triggerings

It was a mistake to do content plugin triggering on the values of textarea / editor fields by default

  • default should have been OFF with a parameter in field configuration to enable it only if needed !

now doing the above is a B/C break ?

@joomla-cms-bot
Copy link

Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/17889

@ghost
Copy link

ghost commented Sep 7, 2017

closed as having Pull Request #17893


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17889.

@ggppdk
Copy link
Contributor

ggppdk commented Sep 7, 2017

There is a need for 2 PRs for this, i am writting the 2nd PR now

@ghost
Copy link

ghost commented Sep 7, 2017

thanks for Info @ggppdk so i let Issue closed. Please comment if it needed be reopened.

@bjxrn
Copy link
Author

bjxrn commented Sep 7, 2017

I just want to add a thanks for you being so quick in not just responding, but also investigating and submitting a PR.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17889.

@ggppdk
Copy link
Contributor

ggppdk commented Sep 7, 2017

@bjxrn

thanks , please test both PRs
#17893
#17895

@bjxrn
Copy link
Author

bjxrn commented Sep 7, 2017

@ggppdk In #17895 on line 191 in administrator/components/com_fields/helpers/fields.php should is_boolean($prepareValue) maybe be is_bool($prepareValue)? is_boolean() gives me an error, is_bool() is a built-in PHP method.

Applying the PRs (and switching is_boolean()/is_bool()) results in a snappy category blog. The example above, where it went from ~300ms to ~10000ms after adding fields, dropped back down to ~500ms after applying the PRs which is a huge improvement. With a different install I tried the same with some media fields, same improvement there.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17889.

@ghost
Copy link

ghost commented Sep 7, 2017

@bjxrn can you please comment on Pull Requests as this is a closed Issue?

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

No branches or pull requests

3 participants