qax-os / excelize

Go language library for reading and writing Microsoft Excel™ (XLAM / XLSM / XLSX / XLTM / XLTX) spreadsheets

BSD 3-Clause "New" or "Revised" License

17.64k stars 1.69k forks source link

PR Details

Avoids large copies when dealing with getSharedFormula, which is called by CalcCellValue, increasing the performance of evaluating formulas in cells.

Description

According to https://stackoverflow.com/a/45786922 uses of range in getSharedFormula caused large copies of objects, which would show-up in flamegraphs as runtime.duffcopy. Changing to a loop with indexing avoids the copying.

Related Issue

1836

Motivation and Context

It cuts out about 40% of execution time from CalcCellValue.

Compared to the one in the issue, the flamegraph now looks like this:

How Has This Been Tested

I've run the patched version inside my program and observed the calculations work as they did before. I've also run go test.

Types of changes

[x] Docs change / refactoring / dependency upgrade
[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[ ] I have added tests to cover my changes.
[x] All new and existing tests passed.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.21%. Comparing base (9d4c2e6) to head (da83b8a).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1837 +/- ## ======================================= Coverage 99.21% 99.21% ======================================= Files 32 32 Lines 23930 23932 +2 ======================================= + Hits 23741 23743 +2 Misses 101 101 Partials 88 88 ``` | [Flag](https://app.codecov.io/gh/qax-os/excelize/pull/1837/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qax-os) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/qax-os/excelize/pull/1837/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qax-os) | `99.21% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qax-os#carryforward-flags-in-the-pull-request-comment) to find out more.