Add Benchmark and minor performance refactor#385
Open
jafin wants to merge 3 commits intoClosedXML:developfrom
Open
Add Benchmark and minor performance refactor#385jafin wants to merge 3 commits intoClosedXML:developfrom
jafin wants to merge 3 commits intoClosedXML:developfrom
Conversation
…lloc drop. Refactor row/column into CellPosition Update expected style in tlist7_horizontal_images Guage test
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmarking Project Setup:
ClosedXML.Report.Benchmarksto measure report generation performance using the BenchmarkDotNet library. (ClosedXML.Report.Benchmarks/Benchmarks/ReportBenchmarks.cs, ClosedXML.Report.Benchmarks/Benchmarks/ReportBenchmarks.csR1-R46)Refactoring:
Refactored
TempSheetBufferto replace individual row and column variables with aCellPositionstruct, simplifying cell position management and improving code readability. (ClosedXML.Report/Excel/TempSheetBuffer.cs, ClosedXML.Report/Excel/TempSheetBuffer.csL13-R135)Updated
SubtotalSummaryFuncto use modern C# features such as target-typednew()and pattern matching, and replaced verbose property definitions with expression-bodied members. (ClosedXML.Report/Excel/SubtotalSummaryFunc.cs, [1] [2] [3]Track
LastCellUsedfor small perf gain.Motivations
We have code that generates spreadsheets, some are taking an increasing amount of time and memory to create. A lot of the performance is in the underlying ClosedXml, not so much reporting lib, but these changes add a slight drop in raw and even slighter performance boost in general.
Benchmark this PR
Benchmark Existing code
Significant amount of ram allocation still!⚠️
Flamegraph shows a lot of time is spent in the underlying ClosedXML lib, one hot path for us is in conditional formatting application. I'll see if I can review that in the ClosedXML project.
Test changes
I amended one expected test, it was failing I Think the change is justified.
Here is the existing test expectation
Here is the amended: note the formatting on the

Item 6 cell. All the other cells follow this pattern, so I think it made sense that this cell also should be rendered the same.I actually think all the cells should have the border formatting, but that is for another day.