Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-36407

Horrible performance if langstringcache is off

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Caching
    • Labels:
    • Testing Instructions:
      Hide
      1. Run unit tests
      2. Log in as an admin
      3. Turn off langstringcache (use the settings search)
      4. Check that your load times for the front page are about normal (not 8s)
      5. Turn on langstringcache
      6. Check that your load times for the back page are about normal (not 8s)
      Show
      Run unit tests Log in as an admin Turn off langstringcache (use the settings search) Check that your load times for the front page are about normal (not 8s) Turn on langstringcache Check that your load times for the back page are about normal (not 8s)
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:
      wip-MDL-36407-m24

      Description

      After MDL-34342 and MDL-36362 were integrated we found those with $CFG->langstringcache off we getting horrible performance.
      This was coming about because the lang string cache uses an ad-hoc cache when that is off and it doesn't know the data is simple.
      As such it inspects the component packs to find out whether it needs to unref things.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            mudrd8mz David Mudrák added a comment -

            I just ran into this as well. Please note that having langstringcache off has been recommended for developers often (in forums etc). Let me suggest to increase the priority of this.

            Show
            mudrd8mz David Mudrák added a comment - I just ran into this as well. Please note that having langstringcache off has been recommended for developers often (in forums etc). Let me suggest to increase the priority of this.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            I agree, this is definitely something to fix before 2.4

            Show
            samhemelryk Sam Hemelryk added a comment - I agree, this is definitely something to fix before 2.4
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Putting this up for peer-review now.

            Show
            samhemelryk Sam Hemelryk added a comment - Putting this up for peer-review now.
            Hide
            timhunt Tim Hunt added a comment -

            Diff URL is a mess. You need to push the master that you based your branch on.

            Show
            timhunt Tim Hunt added a comment - Diff URL is a mess. You need to push the master that you based your branch on.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Is fixed up now Tim, it had been based on integration master in order to resolve conflicts.
            As thats rolled now its based upon stable master and is nice and readable now.

            Thanks
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Is fixed up now Tim, it had been based on integration master in order to resolve conflicts. As thats rolled now its based upon stable master and is nice and readable now. Thanks Sam
            Hide
            timhunt Tim Hunt added a comment -

            +1 from me. You just need to write testing instructions, and then you can submit for integration.

            Show
            timhunt Tim Hunt added a comment - +1 from me. You just need to write testing instructions, and then you can submit for integration.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Tim, up for integration now.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Tim, up for integration now.
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Sam, i've integrated this now.

            Show
            poltawski Dan Poltawski added a comment - Thanks Sam, i've integrated this now.
            Hide
            poltawski Dan Poltawski added a comment -

            Looks good, hurrah!

            Show
            poltawski Dan Poltawski added a comment - Looks good, hurrah!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Many, many thanks for your effort!

            Millions of people will enjoy the results of your work, yay!

            Closing as fixed. Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Many, many thanks for your effort! Millions of people will enjoy the results of your work, yay! Closing as fixed. Ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  3/Dec/12