Micro-optimisation: Avoid Activator.CreateInstance when creating MediaWithCrops - #23496
Conversation
|
Hi there @patrickdemooij9, thank you for this contribution! 👍 While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:
Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution. If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request. Thanks, from your friendly Umbraco GitHub bot 🤖 🙂 |
Activator.CreateInstance when creating MediaWithCrops
Activator.CreateInstance when creating MediaWithCropsActivator.CreateInstance when creating MediaWithCrops
AndyButland
left a comment
There was a problem hiding this comment.
Thanks again @patrickdemooij9 - all looks good. I just pushed a little tidy-up and again some further unit tests, to ensure we have good coverage over this optimisation.
…ediaWithCrops` (#23496) * Cache the MediaWithCrops * Replace with ConstructorInvoker * Removed unused usings. * Code formatting. * Additional test coverage. --------- Co-authored-by: Andy Butland <abutland73@gmail.com>
|
Cherry-picked to |
Prerequisites
Description
As the old comment noted, the creation of the MediaWithCrops is not cached and therefore slow and expensive. Other places in the codebase are already caching this type of creation. My code now also adds that system here. I've also added a benchmark to showcase the change: