Skip to content

Speed up notes helper spec

Peter Leitzen requested to merge pl-spec-helper-notes-perf into master

What does this MR do?

This MR improves the performance of notes helper specs by using let_it_be and before_all.

Related to &3752.

Contributes to https://gitlab.com/gitlab-org/plan/-/issues/145.

Here's the output from test-prof when run with FPROF=1. Note:

  • Total amount of factories created went down from 440 to 59 🚀
  • Total events went down from 11089 to 1185
  • Queries saved: 9904

Before

[TEST PROF INFO] EventProf results for sql.active_record

Total time: 00:12.570 of 00:30.210 (41.61%)
Total events: 11089

Top 5 slowest suites (by time):

NotesHelper (./spec/helpers/notes_helper_spec.rb:5) – 00:12.570 (11089 / 29) of 00:30.210 (41.61%)



Finished in 36.53 seconds (files took 1.68 seconds to load)
29 examples, 0 failures

[TEST PROF INFO] Factories usage

 Total: 440
 Total top-level: 308
 Total time: 17.7832s
 Total uniq factories: 21

   total   top-level     total time      time per call      top-level time               name

     107          87        3.8851s            0.0363s             3.1947s               user
      92           2        3.1230s            0.0339s             0.0694s              issue
      88          88        5.1317s            0.0583s             5.1317s               note
      36          36        4.6807s            0.1300s             4.6807s            project
      29          29        1.3225s            0.0456s             1.3225s              owner
      29          29        0.7050s            0.0243s             0.7050s              group
      19           6        0.6873s            0.0362s             0.2005s          namespace
       8           8        1.1010s            0.1376s             1.1010s      merge_request
       5           5        0.4450s            0.0890s             0.4450s diff_note_on_merge_request
       5           2        0.3086s            0.0617s             0.1329s   personal_snippet
       5           0        0.1654s            0.0331s             0.0000s             author
       3           2        0.0811s            0.0270s             0.0544s    project_snippet
       3           3        0.3474s            0.1158s             0.3474s note_on_personal_snippet
       3           3        0.1355s            0.0452s             0.1355s      note_on_issue
       2           2        0.0464s            0.0232s             0.0464s legacy_diff_note_on_merge_request
       1           1        0.0079s            0.0079s             0.0079s            license
       1           1        0.0265s            0.0265s             0.0265s discussion_note_on_merge_request
       1           1        0.0693s            0.0693s             0.0693s diff_note_on_commit
       1           1        0.0362s            0.0362s             0.0362s legacy_diff_note_on_commit
       1           1        0.0266s            0.0266s             0.0266s discussion_note_on_commit
       1           1        0.0495s            0.0495s             0.0495s note_on_project_snippet

After

[TEST PROF INFO] EventProf results for sql.active_record

Total time: 00:01.966 of 00:07.844 (25.07%)
Total events: 1185

Top 5 slowest suites (by time):

NotesHelper (./spec/helpers/notes_helper_spec.rb:5) – 00:01.966 (1185 / 29) of 00:07.844 (25.07%)



Finished in 13.68 seconds (files took 1.78 seconds to load)
29 examples, 0 failures

[TEST PROF INFO] Factories usage

 Total: 59
 Total top-level: 38
 Total time: 3.9883s
 Total uniq factories: 21

   total   top-level     total time      time per call      top-level time               name

       8           2        0.6499s            0.0812s             0.1306s              issue
       7           3        0.4830s            0.0690s             0.2295s               user
       5           5        0.4810s            0.0962s             0.4810s diff_note_on_merge_request
       5           2        0.3596s            0.0719s             0.1627s   personal_snippet
       5           0        0.1895s            0.0379s             0.0000s             author
       4           4        0.6215s            0.1554s             0.6215s               note
       3           3        0.9424s            0.3141s             0.9424s            project
       3           2        0.1005s            0.0335s             0.0695s    project_snippet
       3           3        0.4811s            0.1604s             0.4811s note_on_personal_snippet
       3           3        0.1468s            0.0489s             0.1468s      note_on_issue
       2           0        0.0973s            0.0486s             0.0000s          namespace
       2           2        0.0693s            0.0347s             0.0693s legacy_diff_note_on_merge_request
       1           1        0.0148s            0.0148s             0.0148s            license
       1           1        0.1753s            0.1753s             0.1753s              owner
       1           1        0.0453s            0.0453s             0.0453s              group
       1           1        0.1753s            0.1753s             0.1753s      merge_request
       1           1        0.0314s            0.0314s             0.0314s discussion_note_on_merge_request
       1           1        0.0790s            0.0790s             0.0790s diff_note_on_commit
       1           1        0.0405s            0.0405s             0.0405s legacy_diff_note_on_commit
       1           1        0.0344s            0.0344s             0.0344s discussion_note_on_commit
       1           1        0.0579s            0.0579s             0.0579s note_on_project_snippet

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Peter Leitzen

Merge request reports