-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add calculation cache to improve CalcCellValue performance #2144
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
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2144 +/- ##
=======================================
Coverage 99.23% 99.23%
=======================================
Files 32 32
Lines 30276 30326 +50
=======================================
+ Hits 30045 30095 +50
Misses 153 153
Partials 78 78
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR. We need add any cache cautiously, after these changes we also need remove cache in following scenario:
- RemoveCol
- RemoveRow
- DuplicateRow
- DuplicateRowTo
- SetSheetCol
And update or delete cache when:
- SetSheetName
- DeleteSheet
Consider if other scenario needed.
Thanks for your quick respond! New additions in this commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code conflicts need resolve.
so sorry, the conflict was caused by my local auto-formatting. I've resolved it by keeping the original content from master |
PR Details
Add calculation cache to improve CalcCellValue performance
Description
Implements a calculation cache mechanism for
CalcCellValue
to significantly improve performance when calculating formulas repeatedly. The cache stores calculated results and automatically invalidates when cells are modified.Related Issue
Fixes #2057 - Performance enhancement - cache calculated values
Motivation and Context
As described in #2057, Excelize doesn't cache calculated values from
CalcCellValue
, causing expensive formulas to be recalculated every time they are referenced by other cells. This leads to poor performance when dealing with complex formula dependencies.Example scenario:
Solution implemented:
sync.Map
SetCellX()
function is called"sheet!cell"
Performance improvement:
How Has This Been Tested
Test coverage includes:
Types of changes
☑️ New feature (non-breaking change which adds functionality)
☐ Docs change / refactoring / dependency upgrade
☐ Bug fix (non-breaking change which fixes an issue)
☐ Breaking change (fix or feature that would cause existing functionality to change)
Checklist
☑️ My code follows the code style of this project.
☐ My change requires a change to the documentation.
☐ I have updated the documentation accordingly.
☑️ I have read the CONTRIBUTING document.
☑️ I have added tests to cover my changes.
☑️ All new and existing tests passed.
Implementation Details
Modified Files:
excelize.go
: Added cache fields (calcCache sync.Map
,calcCacheMu sync.RWMutex
) to File structcalc.go
: Implemented caching logic inCalcCellValue()
+ addedclearCalcCache()
helpercell.go
: Added cache clearing to all 12 Set functionscalc_test.go
: Added 3 comprehensive cache test functions