Patch #33664

evaluate acts_as_activity_provider's scope lazily

Added by Pavel Rosický about 1 month ago. Updated about 1 month ago.

Status:NewStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:Performance
Target version:Candidate for next major release

Description

acts_as_activity_provider's scope is evaluated at require time, we should build the scope after it's actually used

acts_as_activity_provider.patch Magnifier (11.4 KB) Pavel Rosický, 2020-06-24 12:55

History

#1 Updated by Go MAEDA about 1 month ago

Would you tell me how the patch will change the performance or user experience?

#2 Updated by Pavel Rosický about 1 month ago

sure, building a new scope is an expensive operation https://github.com/rails/rails/blob/5-2-stable/activerecord/lib/active_record/scoping/named.rb#L14

in this case, the scope for acts_as_activity_provider should be used after someone asks to get activities on issues for instance

however, right now we have to build a new scope when someone asks to require an issue object. This problem is usually hidden because Rails also does have an autoloading feature (at least in development mode).

before the patch, it does happen
1/ after autoloading an object model, let's say for opening an issue's detail shouldn't be necessary to build a scope for issue's activities, it's still too early
2/ in production mode due to eager loading
3/ if you have a simple plugin with a model patch (like issue_patch.rb). In this case, you have to always load the model (issue.rb) at start-up time

let's defer such operations as late as possible.

after the patch
I do register a proc which doesn't evaluate its content at require time and call it only when we need it (after someone asks for activities)
https://github.com/redmine/redmine/blob/907e0173e40b31d8d0e696b15b18777af98ec9da/lib/plugins/acts_as_activity_provider/lib/acts_as_activity_provider.rb#L54

this patch won't give you a better runtime performance, but it could improve startup time in some situations

note that for the same reason Rails also does define scopes lazily
https://github.com/redmine/redmine/blob/master/app/models/issue.rb#L86
  • correct
    scope :recently_updated, lambda { order(:updated_on => :desc) }
  • wrong
    scope :recently_updated, order(:updated_on => :desc)

that's what is this change about.

#3 Updated by Go MAEDA about 1 month ago

  • Target version set to Candidate for next major release

Also available in: Atom PDF