diff --git a/benchmarks/NPOI.Benchmarks/MergedRegionsBenchmark.cs b/benchmarks/NPOI.Benchmarks/MergedRegionsBenchmark.cs new file mode 100644 index 000000000..e689f141e --- /dev/null +++ b/benchmarks/NPOI.Benchmarks/MergedRegionsBenchmark.cs @@ -0,0 +1,60 @@ +using BenchmarkDotNet.Attributes; +using NPOI.SS.Util; +using NPOI.XSSF.UserModel; + +namespace NPOI.Benchmarks; + +[ShortRunJob] +[MemoryDiagnoser] +public class MergedRegionsBenchmark +{ + private XSSFWorkbook _workbook; + private XSSFSheet _sheet; + + [Params(100, 1000, 5000)] + public int RegionCount { get; set; } + + [GlobalSetup] + public void Setup() + { + _workbook = new XSSFWorkbook(); + _sheet = (XSSFSheet)_workbook.CreateSheet("test"); + // Add non-overlapping merged regions (each row merges columns 0-1) + for (int i = 0; i < RegionCount; i++) + { + _sheet.AddMergedRegionUnsafe(new CellRangeAddress(i, i, 0, 1)); + } + } + + [Benchmark] + public int ReadMergedRegions() + { + // Simulates repeated reads (e.g., during row copy, auto-size, validation) + int count = 0; + for (int i = 0; i < 10; i++) + { + count = _sheet.MergedRegions.Count; + } + return count; + } + + [Benchmark] + public int AddMergedRegions() + { + // Simulates building a sheet with many merged regions (validated) + var tempSheet = (XSSFSheet)_workbook.CreateSheet(); + for (int i = 0; i < RegionCount; i++) + { + tempSheet.AddMergedRegion(new CellRangeAddress(i, i, 0, 1)); + } + int count = tempSheet.MergedRegions.Count; + _workbook.RemoveSheetAt(_workbook.GetSheetIndex(tempSheet)); + return count; + } + + [GlobalCleanup] + public void Cleanup() + { + _workbook?.Dispose(); + } +} diff --git a/ooxml/XSSF/UserModel/XSSFSheet.cs b/ooxml/XSSF/UserModel/XSSFSheet.cs index b2313596d..fb807b585 100644 --- a/ooxml/XSSF/UserModel/XSSFSheet.cs +++ b/ooxml/XSSF/UserModel/XSSFSheet.cs @@ -89,6 +89,7 @@ public partial class XSSFSheet : POIXMLDocumentPart, ISheet private bool _worksheetLoaded = false; private readonly object _loadLock = new object(); internal int _parseCount = 0; + private List _cachedMergedRegions; private CT_Pane Pane { @@ -504,20 +505,23 @@ public List MergedRegions get { EnsureWorksheetLoaded(); - List addresses = new List(); - CT_MergeCells ctMergeCells = worksheet.mergeCells; - if(ctMergeCells == null) + if(_cachedMergedRegions == null) { - return addresses; - } + var addresses = new List(); + CT_MergeCells ctMergeCells = worksheet.mergeCells; + if(ctMergeCells != null) + { + foreach(CT_MergeCell ctMergeCell in ctMergeCells.mergeCell) + { + string ref1 = ctMergeCell.@ref; + addresses.Add(CellRangeAddress.ValueOf(ref1)); + } + } - foreach(CT_MergeCell ctMergeCell in ctMergeCells.mergeCell) - { - string ref1 = ctMergeCell.@ref; - addresses.Add(CellRangeAddress.ValueOf(ref1)); + _cachedMergedRegions = addresses; } - return addresses; + return new List(_cachedMergedRegions); } } @@ -2819,6 +2823,8 @@ public void RemoveMergedRegion(int index) { worksheet.UnsetMergeCells(); } + + InvalidateMergedRegionsCache(); } /// @@ -2833,6 +2839,7 @@ public void RemoveMergedRegions(IList indices) EnsureWorksheetLoaded(); if(!worksheet.IsSetMergeCells()) { + InvalidateMergedRegionsCache(); return; } @@ -2860,6 +2867,8 @@ public void RemoveMergedRegions(IList indices) { ctMergeCells.SetMergeCellArray(newMergeCells.ToArray()); } + + InvalidateMergedRegionsCache(); } /// @@ -4951,6 +4960,7 @@ private int AddMergedRegion(CellRangeAddress region, bool validate) : worksheet.AddNewMergeCells(); CT_MergeCell ctMergeCell = ctMergeCells.AddNewMergeCell(); ctMergeCell.@ref = region.FormatAsString(); + InvalidateMergedRegionsCache(); return ctMergeCells.sizeOfMergeCellArray() - 1; } @@ -5015,6 +5025,11 @@ private void CheckForMergedRegionsIntersectingArrayFormulas() } } + private void InvalidateMergedRegionsCache() + { + _cachedMergedRegions = null; + } + /// /// Verify that candidate region does not intersect with an existing /// merged region in this sheet diff --git a/testcases/ooxml/XSSF/UserModel/TestXSSFSheet.cs b/testcases/ooxml/XSSF/UserModel/TestXSSFSheet.cs index 0aa23c92e..1d4e3cede 100644 --- a/testcases/ooxml/XSSF/UserModel/TestXSSFSheet.cs +++ b/testcases/ooxml/XSSF/UserModel/TestXSSFSheet.cs @@ -624,6 +624,50 @@ public void TestRemoveMergedRegion_lowlevel() workbook.Close(); } + [Test] + public void TestMergedRegionsCacheReturnsFreshList() + { + // Verify that mutating the returned list does not corrupt the cache + XSSFWorkbook workbook = new XSSFWorkbook(); + XSSFSheet sheet = (XSSFSheet)workbook.CreateSheet(); + sheet.AddMergedRegion(new CellRangeAddress(0, 1, 0, 1)); + sheet.AddMergedRegion(new CellRangeAddress(2, 3, 2, 3)); + + var regions1 = sheet.MergedRegions; + ClassicAssert.AreEqual(2, regions1.Count); + + // Mutate the returned list — this should NOT affect subsequent calls + regions1.Add(new CellRangeAddress(10, 11, 10, 11)); + regions1.Clear(); + + var regions2 = sheet.MergedRegions; + ClassicAssert.AreEqual(2, regions2.Count, "Cache should not be corrupted by mutating a previously returned list"); + ClassicAssert.AreEqual("A1:B2", regions2[0].FormatAsString()); + ClassicAssert.AreEqual("C3:D4", regions2[1].FormatAsString()); + + workbook.Close(); + } + + [Test] + public void TestRemoveMergedRegionsOnEmptySheet() + { + // Verify RemoveMergedRegions on a sheet with no merge cells does not throw + // and does not leave stale cache state + XSSFWorkbook workbook = new XSSFWorkbook(); + XSSFSheet sheet = (XSSFSheet)workbook.CreateSheet(); + + // Call RemoveMergedRegions when there are no merged regions at all + sheet.RemoveMergedRegions(new List { 0, 1 }); + ClassicAssert.AreEqual(0, sheet.NumMergedRegions); + + // Now add some regions and verify the cache works correctly after the no-op remove + sheet.AddMergedRegion(new CellRangeAddress(0, 1, 0, 1)); + ClassicAssert.AreEqual(1, sheet.NumMergedRegions); + ClassicAssert.AreEqual("A1:B2", sheet.MergedRegions[0].FormatAsString()); + + workbook.Close(); + } + [Test] public void TestSetDefaultColumnStyle() {