Improve performance of tree rendering in repositories with a lot of items
What does this MR do?
Makes a few minor changes that drastically affect the tree render time in repositories with lots of files in a given directory.
Replaces the use of project_blob_path and project_tree_path with faster (albeit naive) implementations
I noticed in a recent repository tree profile that as much as 60-80% of the request time was spent rendering the tree rows. When I drilled down I found the ActionDispatch::Routing::RouteSet to be suspect.
The Rails *_path methods rely on ActionDispatch::Routing::RouteSet which very inefficiently uses anonymous modules and delegation.
ruby-prof profiles
Before:
After:
wrk latency
Before:
17:30 $ wrk -c 10 -t 1 -d 10 --latency http://localhost:3000/root/lots-of-files?private_token=L7gL84VXdw3_Ke-W2Zpa
Running 10s test @ http://localhost:3000/root/lots-of-files?private_token=L7gL84VXdw3_Ke-W2Zpa
1 threads and 10 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 1.68s 130.51ms 1.88s 66.67%
Req/Sec 8.56 10.13 39.00 81.25%
Latency Distribution
50% 1.63s
75% 1.77s
90% 1.88s
99% 1.88s
27 requests in 10.02s, 15.21MB read
Socket errors: connect 0, read 0, write 0, timeout 18
Requests/sec: 2.69
Transfer/sec: 1.52MB
✔ ~/development/gdk-ce/gitlab [master|✚ 2⚑ 2]
17:31 $ wrk -c 10 -t 1 -d 10 --latency http://localhost:3000/root/lots-of-files?private_token=L7gL84VXdw3_Ke-W2Zpa
Running 10s test @ http://localhost:3000/root/lots-of-files?private_token=L7gL84VXdw3_Ke-W2Zpa
1 threads and 10 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 1.62s 159.94ms 1.97s 69.81%
Req/Sec 10.90 7.52 30.00 53.85%
Latency Distribution
50% 1.59s
75% 1.74s
90% 1.85s
99% 1.97s
56 requests in 10.02s, 31.55MB read
Socket errors: connect 0, read 0, write 0, timeout 3
Requests/sec: 5.59
Transfer/sec: 3.15MBAfter:
20:04 $ wrk -c 1 -t 1 -d 10 --latency http://localhost:3000/root/lots-of-files?private_token=L7gL84VXdw3_Ke-W2Zpa
Running 10s test @ http://localhost:3000/root/lots-of-files?private_token=L7gL84VXdw3_Ke-W2Zpa
1 threads and 1 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 677.62ms 56.82ms 771.18ms 80.00%
Req/Sec 0.62 0.52 1.00 62.50%
Latency Distribution
50% 662.03ms
75% 686.85ms
90% 771.18ms
99% 771.18ms
8 requests in 10.07s, 4.45MB read
Socket errors: connect 0, read 0, write 0, timeout 3
Requests/sec: 0.79
Transfer/sec: 451.92KB
✔ ~/development/gdk-ce/gitlab [tree_perf L|✚ 2⚑ 2]
20:04 $ wrk -c 1 -t 1 -d 10 --latency http://localhost:3000/root/lots-of-files?private_token=L7gL84VXdw3_Ke-W2Zpa
Running 10s test @ http://localhost:3000/root/lots-of-files?private_token=L7gL84VXdw3_Ke-W2Zpa
1 threads and 1 connections
Thread Stats Avg Stdev Max +/- Stdev
Latency 649.59ms 22.21ms 677.00ms 62.50%
Req/Sec 0.80 0.42 1.00 80.00%
Latency Distribution
50% 651.22ms
75% 673.76ms
90% 677.00ms
99% 677.00ms
10 requests in 10.09s, 5.56MB read
Socket errors: connect 0, read 0, write 0, timeout 2
Requests/sec: 0.99
Transfer/sec: 563.82KBTime to first byte reduction
Before:
After:
Are there points in the code the reviewer needs to double check?
Is there any reason we shouldn't use this naive method of building paths for the repository tree?
The risk I see if it we were to change some routes/controllers later on that maybe the path different, but I think that would be fairly obvious and things would fail quickly.
I wonder if there are other risks with encoding or some special way Rails generates routes/paths.
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated -
API support added -
Tests added for this feature/bug - Review
-
Has been reviewed by UX -
Has been reviewed by Frontend -
Has been reviewed by Backend -
Has been reviewed by Database
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Squashed related commits together -
Internationalization required/considered