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

Reduce string allocations when writing documents #1095

Merged
merged 1 commit into from
Jul 22, 2023

Conversation

lahma
Copy link
Collaborator

@lahma lahma commented Jun 10, 2023

string.Format is a bad thing in hot paths. Changed code to call multiple Writes instead of string concatenation.

NPOI.Benchmarks.LargeExcelFileBenchmark

Diff Method Mean Error Allocated
Old Load 11.741 s 1.1257 s 3.65 GB
New 11.626 s (-1%) 2.0913 s 3732.62 MB (0%)
Old Write 3.483 s 0.1738 s 1.19 GB
New 3.058 s (-12%) 0.2296 s 611.83 MB (-50%)
Old Evaluate 62.677 s 4.1891 s 75.32 GB
New 64.658 s (+3%) 24.1520 s 77128.7 MB (0%)

NPOI.Benchmarks.RangeValuesBenchmark

Diff Method Mean Error Allocated
Old Double 85.46 ms 0.367 ms 31.45 MB
New 82.83 ms (-3%) 0.382 ms 26.98 MB (-14%)
Old String 99.93 ms 0.449 ms 59.32 MB
New 91.75 ms (-8%) 0.803 ms 53.92 MB (-9%)
Old Date 114.71 ms 1.603 ms 35.73 MB
New 108.22 ms (-6%) 0.239 ms 30.55 MB (-14%)
Old Formulas 1,064.12 ms 10.889 ms 1480.15 MB
New 1,057.00 ms (-1%) 4.548 ms 1473.42 MB (0%)

@lahma lahma marked this pull request as ready for review June 10, 2023 07:21
@lahma lahma changed the title Reduce string allocations while writing documents Reduce string allocations when writing documents Jun 10, 2023
@tonyqus tonyqus added this to the NPOI 2.6.2 milestone Jun 14, 2023
@tonyqus
Copy link
Member

tonyqus commented Jul 22, 2023

LGTM

@tonyqus tonyqus merged commit c1b836b into nissl-lab:master Jul 22, 2023
@lahma lahma deleted the improve-write-speed branch July 23, 2023 05:55
@tonyqus
Copy link
Member

tonyqus commented Sep 6, 2023

I'm checking if we can rewrite serialization part with https://github.com/Cysharp/ZString.

According to nuget stats, this library looks popular.

I found this library via this post

@lahma
Copy link
Collaborator Author

lahma commented Sep 6, 2023

I can recommend that library, Orchard Core for example uses that, got a PR approved once to that lib Cysharp/ZString#50 .

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

Successfully merging this pull request may close these issues.

2 participants