Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance concerns with getHandlerName #41

Open
gbutt-tr opened this issue Sep 15, 2022 · 8 comments
Open

Performance concerns with getHandlerName #41

gbutt-tr opened this issue Sep 15, 2022 · 8 comments

Comments

@gbutt-tr
Copy link

@kevinohara80 if you're interested, I can create a PR for your review.

We have run into performance issues with getHandlerName. Apparently executing String.valueOf(this) can take a long time to execute if the concrete handler class has a lot of data stored as properties (i.e. storing newList and newMap as properties on your handler class). We have reason to believe this has become more of an issue since Summer 22, but I have no hard evidence on that.

The best solution for us was to replace usage of handlerName Strings with Types throughout the framework. When using types

  • you get the benefits of compile-time errors
  • you optimize the performance of the framework
  • the performance does not vary by use case
  • you eliminate the possibility of throwing a heap exception, which can occur when the property data is too large to convert into a string.

Here are my test results.

Using Types

      Arrangement: handler with property = 200 sobjects, each with a 100k char description
      Call: handler.getHandlerType() 1000 times
      Result Timings: 238ms, 221ms, 194ms, 191ms

Using Strings

      Arrangement: handler with property = 200 sobjects, each with a 100k char description field
      Action: handler.getHandlerName() 1000 times
      Result: System.LimitException: Apex heap size too large: 48042718

      Arrangement: handler with property = 200 sobjects, each with a 10k char description
      Call: handler.getHandlerName() 1000 times
      Result Timings: 6379ms, 6323ms, 6135ms, 6268ms, 10994ms, 10628ms, 6232ms
@dschach
Copy link

dschach commented Jan 4, 2023

@gbutt-tr Would you like to see my version of this in action?
https://github.com/dschach/salesforce-trigger-framework/tree/main/force-app/main/default/classes/TriggerHandler.cls#L210

I'd appreciate any feedback.

@kevinohara80
Copy link
Owner

@gbutt-tr Sorry for the mega-delay. Happy to entertain a PR for this. Thanks!

@rpsherry-starburst
Copy link

rpsherry-starburst commented Jul 26, 2023

@gbutt-tr I raised an issue with getHandlerName() here. Im wondering if my issue could be fixed by using types instead. Can you post a couple examples of how you are using Types?

I found this in your fork
@TestVisible private String getHandlerName() { return this.toString().substringBefore(':'); }

@rpsherry-starburst
Copy link

rpsherry-starburst commented Jul 26, 2023

Just looked through the history of master, saw this got merged. Im updating mine getHandlerName() to match the above code.

@dschach
Copy link

dschach commented Aug 20, 2023

I use
this.handlerName = String.valueOf(this).substring(0, String.valueOf(this).indexOf(':'));
in my version at
https://github.com/dschach/salesforce-trigger-framework/blob/main/force-app/main/default/classes/TriggerHandler.cls

@LuisMoreira95
Copy link

LuisMoreira95 commented Aug 25, 2023

@gbutt-tr @kevinohara80 While exploring this repo, saw this issue. Can anyone, guide me to an article or a programming concept that explains why this solution inproves the performence so dramatically?
Thank you!

@renatoliveira
Copy link
Contributor

@gbutt-tr @kevinohara80 While exploring this repo, saw this issue. Can anyone, guide me to an article or a programming concept that explains why this solution inproves the performence so dramatically? Thank you!

The idea, I believe, is that by using .toString() or String.valueOf(this) you are forcing the interpreter to cast a trigger handler instance into a string, which internally might involve having the interpreter go through all of its attributes at runtime, and those might pose a bottleneck internally. By using the type directly, you are simply making the interpreter use its type directly, not accessing all of its instance attributes or anything like that.

Bear in mind that all of this is speculation though, since Apex is a black box for us.

@LuisMoreira95
Copy link

@gbutt-tr @kevinohara80 While exploring this repo, saw this issue. Can anyone, guide me to an article or a programming concept that explains why this solution inproves the performence so dramatically? Thank you!

The idea, I believe, is that by using .toString() or String.valueOf(this) you are forcing the interpreter to cast a trigger handler instance into a string, which internally might involve having the interpreter go through all of its attributes at runtime, and those might pose a bottleneck internally. By using the type directly, you are simply making the interpreter use its type directly, not accessing all of its instance attributes or anything like that.

Bear in mind that all of this is speculation though, since Apex is a black box for us.

Great Idea, didn't thought about that. Thank you very much Renato!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants