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

Each form leads to many queries for default values #557

Closed fredericalpers closed 1 year ago

fredericalpers commented 1 year ago

Current state

The function onOffice\WPlugin\Field\DefaultValue\DefaultValueRead->readDefaultValuesSingleselect is called approximately 310 times (taking about 400 ms) per form.

A customer had 13 forms on his pages, which lead to loading times high enough that he reported "Service temporarily unavailable" errors.

Investigate

Investigate if the function has to be called that many times. If not please find a fix for a better performance.

Desired state

All performance enhancement is desirable. Ideally the performance should increase drastically.

yeneastgate commented 1 year ago

@fredericalpers. After researching, I found that: "The function onOffice\WPlugin\Field\DefaultValue\DefaultValueRead->readDefaultValuesSingleselect is called approximately 310 times (taking about 400 ms) per form." => The reason: in the code, we are using a "for loop" for all fields returned by the API (of which there are more than 310 fields with type single select). So each " for loop" needs to query once to get into the Database. => So the solution is: Instead of "foreach" all those fields at once ( more than 900 fields according to my data), I will break it down into pieces (each part is 100 fields) to resolve.

Currently, in the code implement " if...else statement" to check for every case for type fields image => I will replace with "switch-case statement" to reduce the complexity of the algorithm. image

Please check with your IT department and give me your opinion on the matter. Thanks!

fredericalpers commented 1 year ago

@yeneastgate I will come back to you with an answer as soon as possible

fredericalpers commented 1 year ago

@yeneastgate Please go ahead and implement the suggested solution. If possible please document the performance improvements before and after.

yeneastgate commented 1 year ago

Please go ahead and implement the suggested solution. If possible please document the performance improvements before and after.

Yes, I got it. Thanks!

yeneastgate commented 1 year ago

@fredericalpers A. Here is a list I have refactored the code for the "default value" feature of all forms to reduce "many queries". Note: The features will be the same as the master branch (as before we refactored).

  1. I have refactored the "default value" feature for "IsRangeField":

  2. I have refactored the "default value" feature for the "isSingleValue" group:

    • date
    • datetime
    • interger
    • float
    • singleselect
    • urn:onoffice-de-ns:smart:2.5:dbAccess:dataType:decimal: do not have a corresponding field,
    • urn:onoffice-de-ns:smart:2.5:dbAccess:dataType:float: do not have a corresponding field,
    • urn:onoffice-de-ns:smart:2.5:dbAccess:dataType:integer: do not have a corresponding field,
    • urn:onoffice-de-ns:smart:2.5:dbAccess:dataType:int: do not have a corresponding field Video in the master branch: https://files.fm/u/awtk9e6q6 Video in branch 38234-each-form-leads-to-many-queries-for-default-values: https://files.fm/u/yzrfamdku
  3. I have refactored the "default value" feature for "isMultiSelect" group. Video in the master branch: https://files.fm/u/29ntsdj9j Video in branch 38234-each-form-leads-to-many-queries-for-default-values: https://files.fm/u/w4gfwr9jj

  4. I have refactored the "default value" feature for "isBoolean" group Video in the master branch: https://files.fm/u/td8q329vr Video in branch 38234-each-form-leads-to-many-queries-for-default-values: https://files.fm/u/5b483582a

  5. I have refactored the "default value" feature for "isStringType" group

    • varchar
    • text
    • urn:onoffice-de-ns:smart:2.5:dbAccess:dataType:varchar
    • urn:onoffice-de-ns:smart:2.5:dbAccess:dataType:Text Video in the master branch: https://files.fm/u/v6ydv73fq Video in branch 38234-each-form-leads-to-many-queries-for-default-values: https://files.fm/u/74qjhd8du
  6. I have refactored the "default value" feature for "isRegZusatz" group.

    • displayAll,
    • displayLive,
    • limitExceeded. => These types do not have a corresponding field, so we do not have an attached video.

B. However, during the test, I found a problem as follows:

=> How do you want to "default value" format for these fields below?

C. The attached photo below shows the performance improvements before and after we refactor the code. Please let me know your opinion about this. Thanks! image

fredericalpers commented 1 year ago

@yeneastgate thank you for the detailed feedback, I will have a look at it asap and let you know :)

fredericalpers commented 1 year ago

These are the fields whose type is not covered in the code. Please check the file listed by name and type as follows: image

=> How do you want to "default value" format for these fields below?

For now we will ignore those specific fields, so we can release the implemented improvements. We will discuss the fields internally on how we want to proceed with them. Thank you :)

yeneastgate commented 1 year ago

For now we will ignore those specific fields, so we can release the implemented improvements. We will discuss the fields internally on how we want to proceed with them. Thank you :)

Ok, I got it. Thanks!