perf: Improve performance of create_package_service_spec
requested to merge gitlab-community/gitlab:428406-improve-performance-create_package_service_spec into master
What does this MR do and why?
- Using
let_it_be
blocks instead oflet
blocks for better test performance - Evaluated performance with FPROF from TestProf, see section below
Performance evaluation
- Reduced time spent in factories from 70 seconds to 8 seconds
- Note: This performance evaluation changed since the creation of this MR because there was a conflict with the master; the collapsed section below shows the previous profiling report
See previous profiling report before the rebase
Before this MR
Test environment set up in 3.070203 seconds
..................................................................................................
Finished in 41.67 seconds (files took 16.66 seconds to load)
98 examples, 0 failures
[TEST PROF INFO] Time spent in factories: 00:27.999 (63.3% of total time)
[TEST PROF INFO] Factories usage
Total: 281
Total top-level: 230
Total time: 00:27.999 (out of 00:52.619)
Total uniq factories: 9
total top-level total time time per call top-level time name
110 98 18.0879s 0.1644s 15.2007s project
110 98 8.8156s 0.0801s 7.9770s namespace
18 18 1.1139s 0.0619s 1.1139s user
12 0 0.0885s 0.0074s 0.0000s ci_stage
12 12 3.6398s 0.3033s 3.6398s ci_build
12 0 3.1838s 0.2653s 0.0000s ci_pipeline
3 3 0.0586s 0.0195s 0.0586s npm_package
3 0 0.0299s 0.0100s 0.0000s package_file
1 1 0.0096s 0.0096s 0.0096s license
After this MR
Test environment set up in 3.953544 seconds
...................................................................................................
Finished in 17.79 seconds (files took 15.92 seconds to load)
99 examples, 0 failures
[TEST PROF INFO] Time spent in factories: 00:03.400 (16.61% of total time)
[TEST PROF INFO] Factories usage
Total: 54
Total top-level: 19
Total time: 00:03.400 (out of 00:28.576)
Total uniq factories: 9
total top-level total time time per call top-level time name
11 3 2.2437s 0.2040s 0.7090s project
11 3 1.1562s 0.1051s 0.6319s namespace
8 0 1.6756s 0.2095s 0.0000s ci_pipeline
8 0 0.0414s 0.0052s 0.0000s ci_stage
8 8 1.9148s 0.2394s 1.9148s ci_build
3 3 0.0766s 0.0255s 0.0766s npm_package
3 0 0.0356s 0.0119s 0.0000s package_file
1 1 0.0561s 0.0561s 0.0561s user
1 1 0.0124s 0.0124s 0.0124s license
BEFORE
Finished in 1 minute 39.85 seconds (files took 16.28 seconds to load)
230 examples, 0 failures
[TEST PROF INFO] Time spent in factories: 01:11.564 (69.93% of total time)
[TEST PROF INFO] Factories usage
Total: 786
Total top-level: 651
Total time: 01:11.564 (out of 01:50.751)
Total uniq factories: 10
total top-level total time time per call top-level time name
263 230 48.3652s 0.1839s 39.6206s project
263 230 21.9584s 0.0835s 19.0626s namespace
132 132 0.8419s 0.0064s 0.8419s package_protection_rule
33 33 10.5124s 0.3186s 10.5124s ci_build
33 0 9.4857s 0.2874s 0.0000s ci_pipeline
33 0 0.1853s 0.0056s 0.0000s ci_stage
22 22 1.4507s 0.0659s 1.4507s user
3 0 0.0366s 0.0122s 0.0000s package_file
3 3 0.0668s 0.0223s 0.0668s npm_package
1 1 0.0093s 0.0093s 0.0093s license
AFTER
Finished in 37.71 seconds (files took 16.12 seconds to load)
230 examples, 0 failures
[TEST PROF INFO] Time spent in factories: 00:08.051 (20.03% of total time)
[TEST PROF INFO] Factories usage
Total: 128
Total top-level: 37
Total time: 00:08.051 (out of 00:48.455)
Total uniq factories: 10
total top-level total time time per call top-level time name
25 3 2.2420s 0.0897s 0.7280s namespace
25 3 6.0661s 0.2426s 1.2878s project
22 22 5.7282s 0.2604s 5.7282s ci_build
22 0 5.1503s 0.2341s 0.0000s ci_pipeline
22 0 0.1202s 0.0055s 0.0000s ci_stage
3 3 0.1760s 0.0587s 0.1760s user
3 3 0.0862s 0.0287s 0.0862s npm_package
3 0 0.0378s 0.0126s 0.0000s package_file
2 2 0.0185s 0.0092s 0.0185s package_protection_rule
1 1 0.0264s 0.0264s 0.0264s license
Screenshots or screen recordings
This MR does not contains frontend changes.
How to set up and validate locally
- Run the improved spec
FPROF=1 bundle exec rspec spec/services/packages/npm/create_package_service_spec.rb
Todos
-
Handle last failing test cases -
Finalize the description of this issue, e.g. comparison of performance, etc.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR. -
Changelog entry added, if necessary -
Documentation created/updated via this MR -
Documentation reviewed by technical writer or follow-up review issue created -
Tests added for this feature/bug -
Tested in all supported browsers -
Conforms to the code review guidelines -
Conforms to the merge request performance guidelines -
Conforms to the style guides -
Conforms to the javascript style guides -
Conforms to the database guides
-
Related to #428406 (closed)
Edited by Gerardo Navarro