Performance optimization - build mapByType Maps as needed about fflib-apex-common HOT 5 CLOSED

Ok, its a thought, given there should be just one commitWork in a request context, i'm considering what portion of this process really consumes compared to the overall execution of the request and the work it doing (querying, doing apex, other trigger code etc). Its always good to be a bit quantitate in doing these type of optimisations, which i know is a little hard on the platform, but the Limits class does permit access to heap and cpu time, so i'd be interested in some comparisons between a base line use case with a full application class and say a stripped down version with a test application class just for that request to get a feel for the variance. As adding extra processing to the register methods overall could have a more detrimental effect. So yeah, i get the idea, just think some profiling is warranted to help give more confidence around such a change. Make sense?

from fflib-apex-common.

Makes perfect sense and completely agree! Based on your suggestion, I created some test cases/scenarios around the concepts. In order to truly get a feel for the difference, I created a customized UOW that approached things the way I described in the OP. Running tests against just larger and smaller Application classes wouldn't really give us a true sense of what the change would mean CPU/Heap/etc. wise, only what having more/less classes does to the current approach.

The tests are very unscientific of course and are focused on pure DML operations not necessarily the relative percentage of an overall transaction that would go through a typical service layer. To your point, if the "DML" is just 5% of an overall transaction and making a change like this means improving the "DML" by 50% but the overall by only 1% (assuming a typical transaction), it might not be worth the investment.

With all that said, here are the two baselines:

1. Existing fflib UOW
2. Modified fflib (using Integer Maps) as described in OP

Test Case Process - Goal here is to mirror a typical UOW usage which I'm guessing normally includes approximately 5-6 records

1. Create a set of records
2. Start Test
3. Log Limits
4. Insert 3 new records using UOW
5. Update 3 records (from #1) using UOW
6. commitWork
7. LogLimits

The results were fairly interesting:

1. UOW with 1 Class
   Current: Heap 5887 / CPU 5
   Modified: Heap 5881 / CPU 7
2. UOW with 100 Classes
   Current: Heap 14331 / CPU 5
   Modified: Heap 9937 / CPU 7
3. UOW with 200 Classes
   Current: Heap 22131 / CPU 6
   Modified: Heap 13737 / CPU 6

Results Summary: The modified approach suggestion makes a fairly significant improvement on Heap usage but (to your concern) negatively impacted CPU slightly. The CPU numbers themselves are rather odd as the numbers are actually higher with smaller classes amounts in one case. To really be able to measure, a true load harness is needed to drive multiple tests, profile different areas at the method level and take averages. Curious if doing this type of performance testing is even possible on SFDC platform?

Conclusion: After writing the modified UOW to follow the new approach, current UOW approach is a little "cleaner" from a coding perspective therefore easier to support/maintain. Given that CPU difference is relatively unchanged (+/-) and even with the improvement in heap usage, I think the best course of action here is to not change the UOW as originally proposed in the OP. Instead, make it a best practice when dealing with "lots of classes" to construct the UOW with a specific class list rather than default to Application.UnitOfWork.newInstance. This could lead to inconsistent usage patterns in the code when "consuming" a UOW, but it's a fairly easy guideline and it keeps the UOW internals easier to support maintain and minimizes requirements on Heap/CPU.

Thoughts?

from fflib-apex-common.

Thanks for this John, great analysis and response! 👍

Yes i agree with your conclusion given your findings.

Though if code breaks away from Application.UnitOfWork.newInstance they also prevent mocking of the UOW via Application.UnitOfWork.setMock. So how about a Application.UnitOfWork.newInstance override that takes the list of SObjectType's?

from fflib-apex-common.

Thanks Andrew, I like the newInstance approach. In reviewing the code, this gets a little dicey and has some margin for error given the newInstance methods are not static and the mock could end having different types than those provided in the newInstance method call. For example, a mock could be set that contains a UOW with 5 SObjects while the newInstance method could be called that has 3 SObjects in the list. If setMock was called, the UOW would have 5 SObjects instead of the expected 3. This can be protected against by the person writing the test taking this in to account or some refactoring of the way this all works can be done. If the SObjectTypes were exposed as a read-only property off the UOW (it's currently a private), some checking could also be done in the newInstance overload.

I've created a pull request that takes the simplest approach. Let me know your thoughts.

Thanks!

from fflib-apex-common.

Thanks, i've merged the PR. 👍

from fflib-apex-common.