Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.3
    • Fix Version/s: 2.4.4
    • Component/s: Caching
    • Labels:
      None

      Description

      The cache::parse_key function takes a fair chunk of time (in my test page it is called 482 times). As part of the MUC infrastructure this should be optimised.

      I didn't find very much wrong with it, but a small detail is that the supports_multiple_identifiers check takes a significant amount of time. It is possible to optimise this (albeit with some duplication) by overriding the function in each of the cache store types for which it always has a fixed value anyway. (I left the mongodb cache alone, because that one's more complicated.)

        Gliffy Diagrams

          Activity

          Hide
          Sam Marshall added a comment -

          I profiled this change twice using the best-of-three method (3 profiles 'before' immediately followed by 3 profiles 'after', compare best of, using wall time for function parse_key):

          21.7ms -> 17.5ms
          21.7ms -> 17.3ms

          So about 4ms. Probably worthwhile as it is a rather safe change and I don't think the duplication is really too horrible...

          Show
          Sam Marshall added a comment - I profiled this change twice using the best-of-three method (3 profiles 'before' immediately followed by 3 profiles 'after', compare best of, using wall time for function parse_key): 21.7ms -> 17.5ms 21.7ms -> 17.3ms So about 4ms. Probably worthwhile as it is a rather safe change and I don't think the duplication is really too horrible...
          Hide
          Tim Hunt added a comment -

          This really needs peer review from Sam H, but +1 from me.

          Show
          Tim Hunt added a comment - This really needs peer review from Sam H, but +1 from me.
          Hide
          Sam Hemelryk added a comment -

          Absolutely. A good change thanks Sam, I've put this up for integration review immediately.

          Show
          Sam Hemelryk added a comment - Absolutely. A good change thanks Sam, I've put this up for integration review immediately.
          Hide
          Dan Poltawski added a comment -

          Integrated to master, thanks Sam

          Show
          Dan Poltawski added a comment - Integrated to master, thanks Sam
          Hide
          Dan Poltawski added a comment -

          Unit tests all good.

          Show
          Dan Poltawski added a comment - Unit tests all good.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Did you think this day was not going to arrive ever?

          Your patience has been rewarded, yay, sent upstream, thanks!

          Closing...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Did you think this day was not going to arrive ever? Your patience has been rewarded, yay, sent upstream, thanks! Closing...ciao

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: