Skip to content

Optimize "cache_key" using a concern

yorickpeterse-staging requested to merge faster-cache-keys into master

What does this MR do?

This MR adds a concern (used by Issue and Note) that provides an optimized version of Rails' cache_key method. See 77c8520e for more details.

Are there points in the code the reviewer needs to double check?

No, though a spell check would be appreciated.

Why was this MR needed?

When loading a lot of data from Redis (e.g. an issue with lots of notes) quite a large amount of time is spent in generating cache keys. This is due to multiple reasons such as:

  • Rails trying to figure out if it should use updated_at or updated_on using somewhat inefficient code
  • Rails relying on pluralization logic to figure out how to generate a cache namespace using a model name
  • Rails calling a whole bunch of methods in general in the process of generating cache keys

In short, Rails is trying to cater to every possible use case, at the cost of performance.

What are the relevant issue numbers?

https://gitlab.com/gitlab-org/gitlab-ce/issues/13651 is not directly related but I ran into this cache_key problem when looking into said issue.

Does this MR meet the acceptance criteria?

Merge request reports