Project

General

Profile

Actions

Patch #33664

closed

evaluate acts_as_activity_provider's scope lazily

Added by Pavel Rosický over 3 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Performance
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

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


Files

Actions #1

Updated by Go MAEDA over 3 years ago

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

Actions #2

Updated by Pavel Rosický over 3 years 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.

Actions #3

Updated by Go MAEDA over 3 years ago

  • Target version set to Candidate for next major release
Actions #4

Updated by Go MAEDA over 3 years ago

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

Setting the target version to 4.2.0.

Actions #5

Updated by Go MAEDA over 3 years ago

Updated the patch to fix some RuboCop offenses.

Actions #6

Updated by Go MAEDA over 3 years ago

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

Committed the patch. Thank you.

Actions

Also available in: Atom PDF