Cache Agent type select options in Agent#new #1804
Conversation
dsander
force-pushed
the
cache-agent-type-dropdown
branch
from
43c7a2c
to
328ac4a
Compare
| @@ -82,6 +82,12 @@ def agent_type_icon(agent, agents) | |||
| end | |||
| end | |||
|
|
|||
| def agent_type_select_options | |||
| Rails.cache.fetch('agent_type_select_options') do | |||
| Agent.types.map {|type| [agent_type_to_human(type.name), type, {title: h(Agent.build_for_type(type.name, User.new(id: 0), {}).html_description.lines.first.strip)}] } | |||
There was a problem hiding this comment.
Can there be anything sensitive in html_description? Do we ever inline stuff for the current user? (Caching is a great idea, so we shouldn't inline anything user-specific!)
There was a problem hiding this comment.
The descriptions could have sensitive information in them, but they do not at the moment. I am not passing the current_user avoid the risk of leaking and checked the first line of the current agent descriptions to make sure they do not include user specific content.
To completely avoid the problem we would need to cache the data per user (which would increase the memory consumption on multi-user environments).
There was a problem hiding this comment.
Can't we just make html_description a class method? description is stored in a class, so you are not supposed to include anything specific to the current user in it anyway.
There was a problem hiding this comment.
There are a couple agents that show things like agent.user_id in there to give you example URLs. We could factor these out, but for now it's not really class level.
There was a problem hiding this comment.
Oh, of course. Now I recall DataOutputAgent!
|
Nice! |
| @@ -25,7 +25,7 @@ | |||
| <% if @agent.new_record? %> | |||
| <div class="form-group type-select"> | |||
| <%= f.label :type %> | |||
| <%= f.select :type, options_for_select([['Select an Agent Type', 'Agent', {title: ''}]] + Agent.types.map {|type| [agent_type_to_human(type.name), type, {title: h(Agent.build_for_type(type.name,current_user,{}).html_description.lines.first.strip)}] }, @agent.type), {}, class: 'form-control', autofocus: true %> | |||
There was a problem hiding this comment.
I think you can just wrap this line with <% cache :agent_select do %> and <% end %> instead of creating a helper method.
There was a problem hiding this comment.
That would also cache the @agent.type.
There was a problem hiding this comment.
I moved [['Select an Agent Type', 'Agent', {title: ''}]] to the helper as well, caching everything with the cache helper does not seem to give significant performance improvements. So I don't think it's worth adding the JavaScript which would be needed to select the Agent type.
Agent#new was by far the slowest action of all controllers. When using many Agent gems the response time goes up to a point where the user starts to wonder if something is going wrong. By caching the Agent description the response time goes down from about 1 second to 100ms in development.
dsander
force-pushed
the
cache-agent-type-dropdown
branch
from
328ac4a
to
6daa6cc
Compare
|
Thanks! |
Agent#new was by far the slowest action of all controllers. When using many Agent gems the response time goes up to a point where the user starts to wonder if something is going wrong. By caching the Agent description the response time goes down from about 1 second to 100ms in development.