Skip to content

N°7636 - Too many OQL requests executed in order to display a lnk #661

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

Merged
merged 9 commits into from
Dec 9, 2024

Conversation

accognet
Copy link
Contributor

Improve performance of displaying links on screen

Currently when displaying a user query, displaying a lnk requires 5 sql queries :
Capture d'écran 2024-07-30 124309

After the PR, the same lnk only requires 2 SQL queries:
Capture d'écran 2024-07-30 124629

In total on the screen, the gain is 12 requests:
before:
Capture d'écran 2024-07-30 124540
after:
Capture d'écran 2024-07-30 124551

@accognet accognet self-assigned this Jul 30, 2024
@accognet accognet added Performance internal Work made by Combodo labels Jul 30, 2024
@rquetiez
Copy link
Contributor

rquetiez commented Dec 2, 2024

Today's review:

  • Count set by default to -1... that's pretty unusual, why not use null instead?
  • Add tests in DBObjectTest, based on AssertDBQueryCount, to show what is being improved at the lowest possible level
  • Using DBObject::GetValues() is not recommended as compared to DBObject::Get(). The former does not give the same results (e.g. lazy loading will lead to missing keys in the array).
  • Improving DBObject::Get() could be preferable, -depending on fix. The root cause that has to be further clarified, with the help of unit tests

@rquetiez
Copy link
Contributor

rquetiez commented Dec 9, 2024

Code review:

  • Requesting rollback on the optimization of MetaModel::GetHyperlink
  • Rename iCount into object_count in the extra params
  • Tests to simplify a little (no need to test GetValue)

@accognet accognet force-pushed the feature/7636-display_a_lnk branch from aa3e801 to fa2e4d6 Compare December 9, 2024 14:13
@rquetiez
Copy link
Contributor

rquetiez commented Dec 9, 2024

Code review : ok for merging

@accognet accognet merged commit 324cb5e into develop Dec 9, 2024
@accognet accognet deleted the feature/7636-display_a_lnk branch December 9, 2024 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Work made by Combodo Performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants