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
    • Rank:
      50138

      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.)

        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: