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

SXSSFWorkbook possible performance overhead #434 #435

Merged
merged 4 commits into from
Dec 8, 2020

Conversation

tony1223
Copy link
Contributor

As title and issue explained.

@tony1223
Copy link
Contributor Author

tony1223 commented Oct 11, 2020

About the firstRowNum issue, reproduce code:

var start = DateTime.UtcNow;
var rowAccessWindowSize = 10000;

var workbook = new SXSSFWorkbook(null, rowAccessWindowSize);

ISheet worksheet = workbook.CreateSheet("Sheet 1");
var maxRow = 10000;
for (int rownum = 0; rownum < maxRow; rownum++)
{
    IRow row = ((SXSSFSheet) worksheet).CreateRow(rownum);
    for (int celnum = 0; celnum < 34; celnum++)
    {
        ICell Cell = row.CreateCell(celnum);
        if (celnum %3 != 0)
        {
            Cell.SetCellValue("Cell: Row-" + rownum + ";CellNo:" + celnum);
        }

        // Console.WriteLine("  ExcelGeneratePerformanceTest: Cell Content :"+rownum+"/"+maxRow +" , "+row.FirstCellNum +","+row.LastCellNum+" "+(DateTime.UtcNow - start).TotalMilliseconds+" ms");
    }

    if (rownum != 0 && rownum % 1000 == 0 )
    {
        ((SXSSFSheet) worksheet).FlushRows();
         Console.WriteLine("  ExcelGeneratePerformanceTest: Content :"+rownum+"/"+maxRow +" , "+worksheet.FirstRowNum+","+worksheet.LastRowNum+" "+(DateTime.UtcNow - start).TotalMilliseconds+" ms");
    }
}

the log result:
ExcelGeneratePerformanceTest: Content :1000/10000 , 1000,0 72.6543 ms |
ExcelGeneratePerformanceTest: Content :2000/10000 , 1000,0 127.5498 ms |
ExcelGeneratePerformanceTest: Content :3000/10000 , 1000,0 192.3763 ms |
ExcelGeneratePerformanceTest: Content :4000/10000 , 1000,0 251.1753 ms |
ExcelGeneratePerformanceTest: Content :5000/10000 , 1000,0 316.0018 ms |
ExcelGeneratePerformanceTest: Content :6000/10000 , 1000,0 379.8311 ms |
ExcelGeneratePerformanceTest: Content :7000/10000 , 1000,0 436.6791 ms |
ExcelGeneratePerformanceTest: Content :8000/10000 , 1000,0 499.5923 ms |
ExcelGeneratePerformanceTest: Content :9000/10000 , 1000,0 562.4246 ms |

The FirstRowNum is locked after first FlushRows() function call,
but LastRowNum not considering the flushedRows (it's zero).

It should be clarify that the two numbers should be in logical (in the view of sheet) or physically (in the view of _rows variable).

The result should be two possible result:
(A)
ExcelGeneratePerformanceTest: Content :1000/10000 , 0,1000
ExcelGeneratePerformanceTest: Content :2000/10000 , 0,2000
ExcelGeneratePerformanceTest: Content :3000/10000 , 0,3000
ExcelGeneratePerformanceTest: Content :4000/10000 , 0,4000
ExcelGeneratePerformanceTest: Content :5000/10000 , 0,5000
ExcelGeneratePerformanceTest: Content :6000/10000 , 0,6000
ExcelGeneratePerformanceTest: Content :7000/10000 , 0,7000
ExcelGeneratePerformanceTest: Content :8000/10000 , 0,8000
ExcelGeneratePerformanceTest: Content :9000/10000 , 0,9000

or
(B)
ExcelGeneratePerformanceTest: Content :1000/10000 , 0,0
ExcelGeneratePerformanceTest: Content :2000/10000 , 0,0
ExcelGeneratePerformanceTest: Content :3000/10000 , 0,0
ExcelGeneratePerformanceTest: Content :4000/10000 , 0,0
ExcelGeneratePerformanceTest: Content :5000/10000 , 0,0
ExcelGeneratePerformanceTest: Content :6000/10000 , 0,0
ExcelGeneratePerformanceTest: Content :7000/10000 , 0,0
ExcelGeneratePerformanceTest: Content :8000/10000 , 0,0
ExcelGeneratePerformanceTest: Content :9000/10000 , 0,0

In the PR it's implement as (A).

@tonyqus tonyqus modified the milestones: NPOI 2.5.3, NPOI 2.5.2 Oct 20, 2020
@tonyqus tonyqus modified the milestones: NPOI 2.5.2, NPOI 2.5.3 Nov 25, 2020
@tonyqus tonyqus modified the milestones: NPOI 2.5.5, NPOI 2.5.3 Dec 8, 2020
@tonyqus tonyqus merged commit 53f62a1 into nissl-lab:master Dec 8, 2020
@tonyqus
Copy link
Member

tonyqus commented Dec 8, 2020

This is a valid fix

@tonyqus tonyqus mentioned this pull request Apr 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants