enums icon indicating copy to clipboard operation
enums copied to clipboard

Optimize InvokableCases to memoize results and remove case looping

Open dhrrgn opened this issue 3 years ago • 3 comments

There is a high likelihood that cases will be invoked many-many times per-request. Currently, it loops over all cases to find the one it needs on each invocation. In cases where the same enum case is invoked hundreds or thousands of times (e.g. when reading DB rows in), this causes unnecessary work. This removes the loop by using reflection, and memoizes the results.

While the performance improvement will likely be unnoticeable, and you could consider this a micro-optimization, it is about 2x as fast as the current implementation. The real benefit is code re-use, as after it finds the enum case, it invokes it so the name/value logic is in 1 place.

dhrrgn avatar Aug 25 '22 19:08 dhrrgn

Could you provide a PR description explaining the changes?

stancl avatar Aug 25 '22 19:08 stancl

@stancl Sorry about that, I had one, not sure what happened. It's updated.

dhrrgn avatar Aug 25 '22 20:08 dhrrgn

Two changes I'd make:

  1. I'm not sure if introducing Reflection code where it previously wasn't needed is a good idea. Perhaps the code is now faster when repeatedly invoking the same case, but I'd guess it's slower on the first call
  2. I'd try to avoid using a static variable. Is it possible to create a property here, or do enums not allow that?

stancl avatar Aug 26 '22 16:08 stancl