Patch #43933
open
Optimize `Issue.load_visible_total_spent_hours` by reusing preloaded spent hours for leaf issues
Description
This patch optimizes Issue.load_visible_total_spent_hours for leaf issues. For a leaf issue, spent_hours and total_spent_hours are always the same because there are no descendants to include in the total. When visible spent_hours has already been preloaded for a leaf issue, the method reuses that value for total_spent_hours instead of running an expensive self-join aggregate query.
This improves performance in code paths that load both spent_hours and total_spent_hours, such as IssuesController#show and IssuesController#index views. In my development environment that uses MySQL, repeated requests to curl http://localhost:3000/issues/1.json with AnonymousUser reduced the query count from 22 to 20, and reduced average ActiveRecord time from 7.32 ms to 6.86 ms, which is about a 6.3% improvement.
Before:
Completed 200 OK in 15ms (Views: 3.0ms | ActiveRecord: 7.0ms (22 queries, 1 cached) | GC: 0.0ms) Completed 200 OK in 30ms (Views: 3.4ms | ActiveRecord: 7.4ms (22 queries, 1 cached) | GC: 14.0ms) Completed 200 OK in 17ms (Views: 3.6ms | ActiveRecord: 7.9ms (22 queries, 1 cached) | GC: 0.6ms) Completed 200 OK in 16ms (Views: 3.1ms | ActiveRecord: 7.4ms (22 queries, 1 cached) | GC: 0.3ms) Completed 200 OK in 16ms (Views: 3.5ms | ActiveRecord: 6.9ms (22 queries, 1 cached) | GC: 0.3ms)
After:
Completed 200 OK in 16ms (Views: 3.4ms | ActiveRecord: 8.0ms (20 queries, 0 cached) | GC: 0.0ms) Completed 200 OK in 13ms (Views: 3.7ms | ActiveRecord: 6.4ms (20 queries, 0 cached) | GC: 0.0ms) Completed 200 OK in 14ms (Views: 3.3ms | ActiveRecord: 6.4ms (20 queries, 0 cached) | GC: 0.0ms) Completed 200 OK in 14ms (Views: 3.2ms | ActiveRecord: 6.1ms (20 queries, 0 cached) | GC: 0.0ms) Completed 200 OK in 15ms (Views: 3.7ms | ActiveRecord: 7.4ms (20 queries, 0 cached) | GC: 0.0ms)
Files
Updated by Go MAEDA 15 days ago
- Target version deleted (
7.0.0)
The patch assumes that if a leaf issue already has @spent_hours, that value can be treated as the visible spent hours and safely reused for @total_spent_hours. However, Issue#spent_hours method also uses the same @spent_hours variable to cache time_entries.sum(:hours), which is the raw total without any permission filtering.
As a result, if spent_hours is called first and stores the raw total in @spent_hours, and then load_visible_total_spent_hours(user) is called, the code may incorrectly assume that @spent_hours already contains the visibility-filtered value produced by load_visible_spent_hours. In that case, total_spent_hours may end up including time entries that are not visible to the user.