onOffice-Web-Org / oo-wp-plugin

onOffice for WP-Websites
https://wp-plugin.onoffice.com
GNU General Public License v3.0
9 stars 9 forks source link

Urgent: Performance Issues Plugin #525

Closed fredericalpers closed 1 year ago

fredericalpers commented 1 year ago

Current state

We have received an increasing number of complaints about performance problems with the plugin.

Both external and internal pages with content of the plugin (mostly property pages) are onwards from version 4.7 significantly slower compared to version 4.6. Change log can be viewed here.

It is noticeable that all customers tested so far have a large to very large database (3.87GB up to 41.80GB). (Screenshots comparing the performance attached)

Investigate

Analysis of the bug on https://46.onofficeweb.com/immobilien/

There seem to be changes for the Custom Labels Estates feature in EstateList.php getFieldLabel($field) from version 4.7. responsible for the poor performance.

In the EstateList.php getFieldLabel($field) method, FieldsCollectionBuilderShort .addFieldsAddressEstateWithRegionValues() is called since this version. Commenting out this line, we can't see any performance issues anymore.

addFieldsAddressEstateWithRegionValues has been around for over a year, but has not been called in getFieldLabel until now. The aim is to find out why this function generates such a large performance loss, and then to find a suitable solution.

Desired state

The performance should be at least as "good" as version 4.6, preferably better and more optimized.

2023-05-17 13_43_35-Documents – EstateList php  onoffice-for-wp-websites-current oOPlugin-modifiedEstateList oOPlugin-current-version

fredericalpers commented 1 year ago

@dai-eastgate @yeneastgate please work on this issue with the highest priority.

dai-eastgate commented 1 year ago

@fredericalpers Yes, we will fix it as soon as possible.

yeneastgate commented 1 year ago

@fredericalpers I have checked the “addFieldsAddressEstateWithRegionValues” function and found out that it is used to pass field type from “multiple select” to “single select” by calling API

See the attached file: image (1)

Therefore, this issue occurs for customers that have many (hundreds or maybe thousands) of regions (regionaler_zusatz). We will continue working to find suitable solution for better performance and more optimized.

So:

Please have a check again and let us know your opinion on our research?

andernath commented 1 year ago

Thanks for analysing @yeneastgate SorryI don't get it, why is this addFieldsAddressEstateWithRegionValues call necessary here in getFieldLabel? I removed it local and still get custom labels of my fields.

For a hotfix please remove it. And maybe we don't need it anyway here?

yeneastgate commented 1 year ago

Thanks for analysing @yeneastgate SorryI don't get it, why is this addFieldsAddressEstateWithRegionValues call necessary here in getFieldLabel? I removed it local and still get custom labels of my fields.

For a hotfix please remove it. And maybe we don't need it anyway here?

I have checked, the function "addFieldsAddressEstateWithRegionValues" is only used to call Region Values and not related to custom labels. I will create a hotfix for this issue

dai-eastgate commented 1 year ago

@fredericalpers @andernath I fixed this issue:

https://github.com/onOffice-Web-Org/oo-wp-plugin/assets/106214469/7cbb5acc-8421-41b3-8aca-0d31e0b14d65

Please review our code and test this issue again.

One more point, it would be great if you could provide us with the test environment which has “a large to very large database (3.87GB up to 41.80GB)” so that we could check the issue correctly. Thanks!

fredericalpers commented 1 year ago

@fredericalpers @andernath I fixed this issue:

  • For the “Custom Labels Estates” feature, I have deleted the “addFieldsAddressEstateWithRegionValues” function. => But the “Custom Labels Estates” feature still work well See the attached video:

    hotfix.mp4

  • For the “Default value for form” and "Filterable for estate list" feature: we must keep it => I have tested those features at the time and they still work well

Please review our code and test this issue again.

One more point, it would be great if you could provide us with the test environment which has “a large to very large database (3.87GB up to 41.80GB)” so that we could check the issue correctly. Thanks!

We are currently reviewing and testing. Also we are waiting for our IT Department to set up a new database with a larger database. Unfortunately the import is not finished yet. We will be able to give you more feedback tomorrow.