Patch #33664

evaluate acts_as_activity_provider's scope lazily

Added by Pavel Rosický 4 months ago. Updated 10 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Performance
Target version:4.2.0

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

acts_as_activity_provider-v2.patch Magnifier (9.92 KB) Go MAEDA, 2020-09-28 11:32

Associated revisions

Revision 20148
Added by Go MAEDA 10 days ago

Evaluate acts_as_activity_provider's scope lazily (#33664).

Patch by Pavel Rosický.

History

#1 Updated by Go MAEDA 4 months ago

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

#2 Updated by Pavel Rosický 4 months 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 4 months ago

  • Target version set to Candidate for next major release

#4 Updated by Go MAEDA about 1 month ago

  • Target version changed from Candidate for next major release to 4.2.0

Setting the target version to 4.2.0.

#5 Updated by Go MAEDA 29 days ago

Updated the patch to fix some RuboCop offenses.

#6 Updated by Go MAEDA 10 days ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA

Committed the patch. Thank you.

Also available in: Atom PDF