PR: Cache active personnel and a person's Advanced AsTech Contribution#8520
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8520 +/- ##
============================================
+ Coverage 12.25% 12.29% +0.03%
- Complexity 7373 7398 +25
============================================
Files 1287 1287
Lines 165194 165323 +129
Branches 24847 24879 +32
============================================
+ Hits 20247 20323 +76
- Misses 143016 143054 +38
- Partials 1931 1946 +15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR introduces caching mechanisms to optimize performance-critical operations in MekHQ by caching the list of active personnel and individual person's Advanced AsTech contributions. According to the PR description, these changes address significant performance issues where processing a single day was taking over a minute in large campaigns.
Key Changes:
- Added caching for active personnel lists in Campaign class with different filter combinations
- Moved and cached the Advanced AsTech contribution calculation from Campaign to Person class
- Added an event handler to invalidate caches when personnel data changes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| MekHQ/src/mekhq/campaign/Campaign.java | Added activePersonnelCache Map, modified getActivePersonnel() to use cache, added handlePersonUpdate() event handler, changed ASSISTANT_SKILL_LEVEL_DIVIDER visibility to public, simplified getAdvancedAsTechContribution() to delegate to Person |
| MekHQ/src/mekhq/campaign/personnel/Person.java | Added transient advancedAsTechContribution cache field, implemented getAdvancedAsTechContribution() method with caching logic, added cache invalidation methods |
Comments suppressed due to low confidence (1)
MekHQ/src/mekhq/campaign/personnel/Person.java:8772
- @param tag "person" does not match any actual parameter of method "getAdvancedAsTechContribution()".
* @param person the {@link Person} whose contribution is to be calculated
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I have concerns about this. Raising on Discord |
Those are some expensive calls.
We get a list of all active personnel very often. This list doesn't change very often. Let's cache it to improve performance. SAme with the Advanced AsTech contribution.
Prior to this change, the save in #8496 was taking over a minute to process one day, with a lot of that time being spent getting the amount of AsTech contribution and the list of active personnel. This makes both of those lookups near instant.
To ensure the caches are properly cleared, I've set it to be
@Subscribe'd toPersonEvent. Any changes to a person will invalidate the personnel list caches, and clear the person's cached advanced AsTech contribution.