Skip to content

Commit

Permalink
Build sort if needed.
Browse files Browse the repository at this point in the history
  • Loading branch information
@steveyken
steveyken committed Dec 20, 2012
1 parent a8579fb commit 55f881e
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion app/controllers/entities_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def get_users

def ransack_search
@ransack_search ||= load_ransack_search
@ransack_search.build_sort
@ransack_search.build_sort if @ransack_search.sorts.empty?
@ransack_search
end

Expand Down

5 comments on commit 55f881e

@CloCkWeRX
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its good to have a check if it's needed, but this unfortunately it still hurts performance in some scenarios.
For example, we're importing a lot of data to the Accounts, which means the search index gets rebuilt. Not being obvious, this was excluded from my rake task...

... and then viewing an index page suddenly takes minutes and does a lot of writes for a GET request, due to this and the before_filter .

IMO, ransack UI is doin' it wrong - it would be better to shift this to happening at the CRUD controller level for entities, or perhaps an after_save hook on the model.

@steveyken
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding @ndbroadbent to discussion

I've done a bit of benchmarking and the culprit of the slow search load is the 'attribute_select' function in ransack (not ransack_ui). See https://github.com/ernie/ransack/blob/v0.7.2/lib/ransack/helpers/form_builder.rb#L21 There's already a ransack issue open for this but doesn't seem to have been resolved yet. See activerecord-hackery/ransack#154

I'm interested in what you mean by the index page doing a lot of GET requests... this doesn't sound like typical behaviour?

@steveyken
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: the 'attribute_collection_for_base' seems to be eating the time, This is called in attribute_select. Here's a benchmark:

attribute_collection_for_base (306.1ms)
attribute_collection_for_base (140.8ms)
attribute_collection_for_base (254.7ms)
attribute_collection_for_base (70.7ms)
attribute_collection_for_base (61.0ms)
attribute_collection_for_base (73.9ms)
attribute_collection_for_base (42.0ms)
attribute_collection_for_base (21.1ms)
attribute_collection_for_bases (978.5ms)

@steveyken
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attribute_collection_for_bases

Looks like translation is being overused...? A lot of calls and taking the time. Perhaps there is a more efficient way to code this?

@CloCkWeRX
Copy link
Member

@CloCkWeRX CloCkWeRX commented on 55f881e Jun 19, 2013 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.