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

Each form leads to many queries for default values #557

Closed
fredericalpers opened this issue Jun 12, 2023 · 8 comments · Fixed by #604
Closed

Each form leads to many queries for default values #557

fredericalpers opened this issue Jun 12, 2023 · 8 comments · Fixed by #604
Labels
QA Issue or Pull request that is in review
Milestone

Comments

@fredericalpers
Copy link
Member

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.

@fredericalpers fredericalpers added this to the v4.16 milestone Jun 12, 2023
@onOffice-Web-Org onOffice-Web-Org locked and limited conversation to collaborators Jun 12, 2023
@fredericalpers fredericalpers modified the milestones: v4.16, v4.15 Jun 19, 2023
@onOffice-Web-Org onOffice-Web-Org unlocked this conversation Jun 20, 2023
@yeneastgate
Copy link
Contributor

yeneastgate commented Jul 27, 2023

@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
Copy link
Member Author

fredericalpers commented Jul 27, 2023

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

@fredericalpers
Copy link
Member Author

fredericalpers commented Jul 31, 2023

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

@yeneastgate
Copy link
Contributor

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

Yes, I got it. Thanks!

@yeneastgate
Copy link
Contributor

@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":
  1. 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
  1. 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

  2. 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

  3. 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
  1. 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:

  • Some fields are not displayed in the correct format "default value" feature for that field type.
    image

  • The reason is that the "field type" API result responses of these fields are not covered in the source code.
    image

  • 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?

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
Copy link
Member Author

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

@fredericalpers
Copy link
Member Author

fredericalpers commented Aug 16, 2023

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
Copy link
Contributor

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!

@fredericalpers fredericalpers added QA Issue or Pull request that is in review and removed 1 week labels Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA Issue or Pull request that is in review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants