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

Optimize the very inefficient database queries in the deviation list view #1302

Open
markkuriekkinen opened this issue Oct 31, 2023 · 1 comment · Fixed by #1304
Open

Optimize the very inefficient database queries in the deviation list view #1302

markkuriekkinen opened this issue Oct 31, 2023 · 1 comment · Fixed by #1304
Labels
area: performance Related to the performance of the system effort: days Estimated to take less than one week, from the creation of a new branch to the merging experience: beginner required knowledge estimate O1 needs Requested in the Programming 1 course requester: CS The issue is raised internally by a CS teacher type: bug This is a bug

Comments

@markkuriekkinen
Copy link
Contributor

#1122 presented performance issues in the deviations list page. This new issue focuses on specific performance issues.

Using the Django Debug toolbar, we have noticed that the database queries in the deviations list page include some very inefficient parts.

  • {{ deviations.0.exercise.course_module }}
    • rendering the module's name causes new queries for each record because it needs to load the course instance for the module numbering settings (that set Arabic or Roman numerals). Has select_related() or prefetch_related() been used incorrectly since the code tries to prefetch the course instances?
    • ordered_deviations = (
      all_deviations
      .select_related(
      'submitter', 'submitter__user',
      'granter', 'granter__user',
      'exercise', 'exercise__course_module',
      )
      # parent is prefetched because there may be multiple ancestors, and
      # they are needed for building the deviation's URL.
      .prefetch_related('exercise__parent')
      .order_by('submitter', 'exercise__course_module')
      )
    • does the variable in the template benefit from the prefetching in the queryset or is the queryset's cache somehow lost when the template is rendered in this case? Does the variable refer to the original queryset anymore at that stage?
  • all_deviations = self.deviation_model.objects.filter(
    exercise__course_module__course_instance=self.instance
    )
    • when all deviations in the course are queried, the query includes a lot of unnecessary attributes. Loading them from the database consumes a lot of resources. Especially the large JSON fields in the exercise model consume a lot of bandwidth even though those attributes are not used at all in the deviations list.
    • Unwanted attributes can be avoided with the Django queryset methods only() and defer().
    • exercise.exercise_info is one of the unnecessary attributes in this case.
@markkuriekkinen markkuriekkinen added type: bug This is a bug O1 needs Requested in the Programming 1 course effort: days Estimated to take less than one week, from the creation of a new branch to the merging experience: beginner required knowledge estimate requester: CS The issue is raised internally by a CS teacher area: performance Related to the performance of the system labels Oct 31, 2023
ihalaij1 added a commit to ihalaij1/a-plus that referenced this issue Nov 7, 2023
The course module name is no longer queried separately for
each deviation. This reduces the number of performed
queries significantly. Additionally, some large unnecessary
attributes are no longer loaded.

Fixes apluslms#1302
ihalaij1 added a commit to ihalaij1/a-plus that referenced this issue Nov 7, 2023
The course module name is no longer queried separately for
each deviation. This reduces the number of performed
queries significantly. Additionally, some large unnecessary
attributes are no longer loaded.

Fixes apluslms#1302
ihalaij1 added a commit to ihalaij1/a-plus that referenced this issue Nov 13, 2023
The course module name is no longer queried separately for
each deviation. This reduces the number of performed
queries significantly. Additionally, some large unnecessary
attributes are no longer loaded.

Fixes apluslms#1302
ihalaij1 added a commit that referenced this issue Nov 13, 2023
The course module name is no longer queried separately for
each deviation. This reduces the number of performed
queries significantly. Additionally, some large unnecessary
attributes are no longer loaded.

Fixes #1302
murhum1 pushed a commit to murhum1/a-plus that referenced this issue Dec 21, 2023
The course module name is no longer queried separately for
each deviation. This reduces the number of performed
queries significantly. Additionally, some large unnecessary
attributes are no longer loaded.

Fixes apluslms#1302
ihalaij1 added a commit to ihalaij1/a-plus that referenced this issue Dec 22, 2023
The course module name is no longer queried separately for
each deviation. This reduces the number of performed
queries significantly. Additionally, some large unnecessary
attributes are no longer loaded.

Fixes apluslms#1302
ihalaij1 added a commit to ihalaij1/a-plus that referenced this issue Dec 22, 2023
The course module name is no longer queried separately for
each deviation. This reduces the number of performed
queries significantly. Additionally, some large unnecessary
attributes are no longer loaded.

Fixes apluslms#1302
@ihalaij1 ihalaij1 reopened this Feb 1, 2024
@ihalaij1
Copy link
Contributor

ihalaij1 commented Feb 1, 2024

Reopened since the page still doesn't load quickly enough.

@ihalaij1 ihalaij1 removed their assignment Feb 6, 2024
@murhum1 murhum1 removed their assignment May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: performance Related to the performance of the system effort: days Estimated to take less than one week, from the creation of a new branch to the merging experience: beginner required knowledge estimate O1 needs Requested in the Programming 1 course requester: CS The issue is raised internally by a CS teacher type: bug This is a bug
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

3 participants