perf: add lru_cache to improve performance with multiple themes

[Alec4r]

These changes should improve the performance when there are plenty of themes defined in COMPREHENSIVE_THEME_DIRS, using lru_cache to reduce the number of calls to the method is_theme_dir.

We have a bad response time due to the file I/O when it's running in docker the more themes you add to your platform the more slow answer you will have.

We can evidence this in these tests:

Test without comprehensive theming (0 Themes)

running (10m09.6s), 00/28 VUs, 602 complete and 0 interrupted iterations
http_req_duration..............: avg=553.57ms min=107.08ms med=492.97ms max=3.98s p(90)=1.02s p(95)=1.24s

Test with indigo (1 Theme)

running (10m15.3s), 00/28 VUs, 523 complete and 0 interrupted iterations
http_req_duration..............: avg=1.24s min=132.26ms med=1.23s max=2.95s p(90)=2s p(95)=2.17s

Test with multiple themes (5 Themes)

running (10m12.7s), 00/28 VUs, 313 complete and 3 interrupted iterations
http_req_duration..............: avg=4.56s   min=111.86ms med=5.25s   max=7.57s    p(90)=6.25s    p(95)=6.5s

Finally, we can see the improvement with the changes in this PR (5 Themes)

running (10m06.6s), 00/28 VUs, 592 complete and 0 interrupted iterations
http_req_duration..............: avg=633.36ms min=101.13ms med=606.9ms  max=2.15s    p(90)=1.13s    p(95)=1.27s 

For a thorough description of the issue, performance tests & solution design, you could refer to
ISSUE: Comprehensive theming performance

---

We may wish to backport all of these changes to Nutmeg

[openedx-webhooks]

Thanks for the pull request, @Alec4r! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[ormsbee]

FYI @openedx/perf-interest: This is a great perf win, but the bigger takeaway here is that the switch to docker-based deployment looks like it's causing disk I/O to become much more expensive, meaning that we could see performance regressions in a bunch of places that we haven't touched in a long time.

But enough sober talk. Look at the sweet numbers!

Before:

avg=3.17s
min=133.3ms
med=3.56s
max=7.38s
p(90)=4.54s
p(95)=4.8s

After:

avg=633.36ms
min=101.13ms
med=606.9ms 
max=2.15s
p(90)=1.13s
p(95)=1.27s 

[ormsbee]

The code looks fine (after tests pass again), but a few requests on the commit message:

1. Please use perf: instead of feat: for this. (For the full list of possible types, please see the "Type" section of OEP-51.
2. Please take the context you give in your PR description and add it to your commit message.
3. You might want to also mention that it's the file I/O that's causing issues, that this came up when running in Docker, and that it saves thousands of calls to listdir (?) when running with a handful of themes.

Having external links is fine, but please add enough context to your commit message so that it's clear why this was done even if the link breaks later. Ten years from now, we might no longer be using the current forums, or even GitHub. But the commit message will live on.

[ormsbee]

BTW, I think the tests should be able to clear the LRU cache with calls like get_themes.clear_cache() (the @lru_cache decorator adds this to functions it wraps).

[mariajgrimaldi]

I can't approve and run your test. Is there something wrong with the runner?

[kdmccormick]

GHA is reporting an error: https://github.com/openedx/edx-platform/actions/runs/3190974009

It says You have an error in your yaml syntax on line 65

I think this is an error on master, I'll take a look.

[kdmccormick]

Yup, the error is from master. Once this fix merges, you can rebase and then tests should run.

[kdmccormick]

Merged the CI fix, @Alec4r you should be good to rebase now.

[ormsbee]

Not sure why I had to authorize the workflows a second time, but I'll merge once tests pass.

[mariajgrimaldi]

@ormsbee: not sure either. But each time a first-time contributor opens a PR, someone with permissions needs to approve the workflow each time a new commit is pushed. Not sure what the conditions are for this to happen, but it has happened to me a couple of times.

[Alec4r]

@ormsbee the tests are passing, we can continue with the merge?

[openedx-webhooks]

@Alec4r 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.

[ormsbee]

@Alec4r: Please add a note about this to the Olive release notes page under the Operational section. Anybody running a Docker-based deployment with theming will likely be interested in this.