Bug 51635 - Improved performance of XSSFSheet#write

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1156272 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 26 insertions, 45 deletions

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 34733a2a44..1633fdc485 100644
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.8-beta4" date="2011-??-??">
+           <action dev="poi-developers" type="fix">51635 - Improved performance of XSSFSheet#write</action>
            <action dev="poi-developers" type="fix">47731 - Word Extractor considers text copied from some website as an embedded object</action>
            <action dev="poi-developers" type="add">Add Word-to-Text converter and use it as replacement for WordExtractor</action>
            <action dev="poi-developers" type="fix">51604 - replace text fails for doc ( poi 3.8 beta release from download site )</action>
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
index 573777e560..a78162917b 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
@@ -570,7 +570,15 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
             ctRow = prev.getCTRow();
             ctRow.set(CTRow.Factory.newInstance());
         } else {
-            ctRow = worksheet.getSheetData().addNewRow();
+        	if(_rows.isEmpty() || rownum > _rows.lastKey()) {
+        		// we can append the new row at the end
+        		ctRow = worksheet.getSheetData().addNewRow();
+        	} else {
+        		// get number of rows where row index < rownum
+        		// --> this tells us where our row should go
+        		int idx = _rows.headMap(rownum).size();
+        		ctRow = worksheet.getSheetData().insertNewRow(idx);
+        	}
         }
         XSSFRow r = new XSSFRow(ctRow, this);
         r.setRowNum(rownum);
@@ -1496,7 +1504,9 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
 
         for(XSSFCell cell : cellsToDelete) row.removeCell(cell);
 
+        int idx = _rows.headMap(row.getRowNum()).size();
         _rows.remove(row.getRowNum());
+        worksheet.getSheetData().removeRow(idx);
     }
 
     /**
@@ -2355,7 +2365,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
      */
     @SuppressWarnings("deprecation") //YK: getXYZArray() array accessors are deprecated in xmlbeans with JDK 1.5 support
     public void shiftRows(int startRow, int endRow, int n, boolean copyRowHeight, boolean resetOriginalRowHeight) {
-        for (Iterator<Row> it = rowIterator() ; it.hasNext() ; ) {
+    	for (Iterator<Row> it = rowIterator() ; it.hasNext() ; ) {
             XSSFRow row = (XSSFRow)it.next();
             int rownum = row.getRowNum();
             if(rownum < startRow) continue;
@@ -2365,6 +2375,10 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
             }
 
             if (removeRow(startRow, endRow, n, rownum)) {
+            	// remove row from worksheet.getSheetData row array
+            	int idx = _rows.headMap(row.getRowNum()).size();
+                worksheet.getSheetData().removeRow(idx);
+                // remove row from _rows
                 it.remove();
             }
             else if (rownum >= startRow && rownum <= endRow) {
@@ -2689,7 +2703,6 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
         for(XSSFRow row : _rows.values()){
             row.onDocumentWrite();
         }
-        ensureRowOrdering();
 
         XmlOptions xmlOptions = new XmlOptions(DEFAULT_XML_OPTIONS);
         xmlOptions.setSaveSyntheticDocumentElement(new QName(CTWorksheet.type.getName().getNamespaceURI(), "worksheet"));
@@ -2701,39 +2714,6 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
     }
 
     /**
-     * ensure that the array of CTRow written to CTSheetData is ordered by row index
-     */
-    private void ensureRowOrdering(){
-        CTSheetData sheetData = worksheet.getSheetData();
-        // check if row indexes in CTSheetData match the internal model:
-        // rows in the internal model (_rows) are always ordered while
-        // CTRow beans held by CTSheetData may be not, for example, user can
-        // insert rows in random order, shift rows after insertion, etc.
-        boolean isOrdered = true;
-        if(sheetData.sizeOfRowArray() != _rows.size()) isOrdered = false;
-        else {
-            int i = 0;
-            for (XSSFRow row : _rows.values()) {
-                CTRow c1 = row.getCTRow();
-                CTRow c2 = sheetData.getRowArray(i++); 
-                if (c1.getR() != c2.getR()){
-                    isOrdered = false;
-                    break;
-                }
-            }
-        }
-        
-        if(!isOrdered){
-            CTRow[] cArray = new CTRow[_rows.size()];
-            int i = 0;
-            for(XSSFRow row : _rows.values()){
-                cArray[i++] = row.getCTRow();
-            }
-            sheetData.setRowArray(cArray);
-        }
-    }
-
-    /**
      * @return true when Autofilters are locked and the sheet is protected.
      */
     public boolean isAutoFilterLocked() {
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java
index aebdd45a7a..ad02fa69e1 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java
@@ -901,7 +901,7 @@ public final class TestXSSFSheet extends BaseTestSheet {
 
     /**
      * Rows and cells can be created in random order,
-     * but serialization forces strict ascending order of the CTRow and CTCell xml beans
+     * but CTRows are kept in ascending order
      */
     public void testCreateRow() {
         XSSFWorkbook workbook = new XSSFWorkbook();
@@ -929,20 +929,20 @@ public final class TestXSSFSheet extends BaseTestSheet {
         CTRow[] xrow = sheetData.getRowArray();
         assertEquals(3, xrow.length);
 
-        //rows are unsorted: {2, 1, 0}
-        assertEquals(2, xrow[0].sizeOfCArray());
-        assertEquals(3, xrow[0].getR());
-        assertTrue(xrow[0].equals(row1.getCTRow()));
+        //rows are sorted: {0, 1, 2}
+        assertEquals(4, xrow[0].sizeOfCArray());
+        assertEquals(1, xrow[0].getR());
+        assertTrue(xrow[0].equals(row3.getCTRow()));
 
         assertEquals(3, xrow[1].sizeOfCArray());
         assertEquals(2, xrow[1].getR());
         assertTrue(xrow[1].equals(row2.getCTRow()));
 
-        assertEquals(4, xrow[2].sizeOfCArray());
-        assertEquals(1, xrow[2].getR());
-        assertTrue(xrow[2].equals(row3.getCTRow()));
+        assertEquals(2, xrow[2].sizeOfCArray());
+        assertEquals(3, xrow[2].getR());
+        assertTrue(xrow[2].equals(row1.getCTRow()));
 
-        CTCell[] xcell = xrow[2].getCArray();
+        CTCell[] xcell = xrow[0].getCArray();
         assertEquals("D1", xcell[0].getR());
         assertEquals("A1", xcell[1].getR());
         assertEquals("C1", xcell[2].getR());