This fix performance impact introduced in #1692 (#1849)

Co-authored-by: Nchun.zhang2 <chun.zhang2@geely.com>

- This fix speed slowdown and memory usage increase base on the reverts commit 6220a798
- Fix panic on read workbook with internal row element without r attribute
- Update the unit tests

adjust.go

@@ -201,13 +201,13 @@ func (f *File) adjustRowDimensions(sheet string, ws *xlsxWorksheet, row, offset
 		return nil
 	}
 	lastRow := &ws.SheetData.Row[totalRows-1]
-	if newRow := *lastRow.R + offset; *lastRow.R >= row && newRow > 0 && newRow > TotalRows {
+	if newRow := lastRow.R + offset; lastRow.R >= row && newRow > 0 && newRow > TotalRows {
 		return ErrMaxRows
 	}
 	numOfRows := len(ws.SheetData.Row)
 	for i := 0; i < numOfRows; i++ {
 		r := &ws.SheetData.Row[i]
-		if newRow := *r.R + offset; *r.R >= row && newRow > 0 {
+		if newRow := r.R + offset; r.R >= row && newRow > 0 {
 			r.adjustSingleRowDimensions(offset)
 		}
 		if err := f.adjustSingleRowFormulas(sheet, sheet, r, row, offset, false); err != nil {
@@ -219,10 +219,10 @@ func (f *File) adjustRowDimensions(sheet string, ws *xlsxWorksheet, row, offset
 
 // adjustSingleRowDimensions provides a function to adjust single row dimensions.
 func (r *xlsxRow) adjustSingleRowDimensions(offset int) {
-	r.R = intPtr(*r.R + offset)
+	r.R += offset
 	for i, col := range r.C {
 		colName, _, _ := SplitCellName(col.R)
-		r.C[i].R, _ = JoinCellName(colName, *r.R)
+		r.C[i].R, _ = JoinCellName(colName, r.R)
 	}
 }
 
@@ -701,7 +701,7 @@ func (f *File) adjustAutoFilter(ws *xlsxWorksheet, sheet string, dir adjustDirec
 		ws.AutoFilter = nil
 		for rowIdx := range ws.SheetData.Row {
 			rowData := &ws.SheetData.Row[rowIdx]
-			if rowData.R != nil && *rowData.R > y1 && *rowData.R <= y2 {
+			if rowData.R > y1 && rowData.R <= y2 {
 				rowData.Hidden = false
 			}
 		}

adjust_test.go

@@ -289,7 +289,7 @@ func TestAdjustAutoFilter(t *testing.T) {
 	f := NewFile()
 	assert.NoError(t, f.adjustAutoFilter(&xlsxWorksheet{
 		SheetData: xlsxSheetData{
-			Row: []xlsxRow{{Hidden: true, R: intPtr(2)}},
+			Row: []xlsxRow{{Hidden: true, R: 2}},
 		},
 		AutoFilter: &xlsxAutoFilter{
 			Ref: "A1:A3",

cell.go

@@ -1426,8 +1426,8 @@ func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c
 		return "", err
 	}
 	lastRowNum := 0
-	if l := len(ws.SheetData.Row); l > 0 && ws.SheetData.Row[l-1].R != nil {
-		lastRowNum = *ws.SheetData.Row[l-1].R
+	if l := len(ws.SheetData.Row); l > 0 {
+		lastRowNum = ws.SheetData.Row[l-1].R
 	}
 
 	// keep in mind: row starts from 1
@@ -1437,7 +1437,7 @@ func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c
 
 	for rowIdx := range ws.SheetData.Row {
 		rowData := &ws.SheetData.Row[rowIdx]
-		if rowData.R != nil && *rowData.R != row {
+		if rowData.R != row {
 			continue
 		}
 		for colIdx := range rowData.C {

cell_test.go

@@ -278,7 +278,7 @@ func TestSetCellValue(t *testing.T) {
 	f.Pkg.Store(defaultXMLPathSharedStrings, []byte(fmt.Sprintf(`<sst xmlns="%s" count="2" uniqueCount="1"><si><t>a</t></si><si><t>a</t></si></sst>`, NameSpaceSpreadSheet.Value)))
 	f.Sheet.Store("xl/worksheets/sheet1.xml", &xlsxWorksheet{
 		SheetData: xlsxSheetData{Row: []xlsxRow{
-			{R: intPtr(1), C: []xlsxC{{R: "A1", T: "str", V: "1"}}},
+			{R: 1, C: []xlsxC{{R: "A1", T: "str", V: "1"}}},
 		}},
 	})
 	assert.NoError(t, f.SetCellValue("Sheet1", "A1", "b"))
@@ -387,6 +387,9 @@ func TestGetCellValue(t *testing.T) {
 	f.Sheet.Delete("xl/worksheets/sheet1.xml")
 	f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `<row r="0"><c r="H6" t="inlineStr"><is><t>H6</t></is></c><c r="A1" t="inlineStr"><is><t>r0A6</t></is></c><c r="F4" t="inlineStr"><is><t>F4</t></is></c></row><row><c r="A1" t="inlineStr"><is><t>A6</t></is></c><c r="B1" t="inlineStr"><is><t>B6</t></is></c><c r="C1" t="inlineStr"><is><t>C6</t></is></c></row><row r="3"><c r="A3"><v>100</v></c><c r="B3" t="inlineStr"><is><t>B3</t></is></c></row>`)))
 	f.checked = sync.Map{}
+	cell, err = f.GetCellValue("Sheet1", "H6")
+	assert.Equal(t, "H6", cell)
+	assert.NoError(t, err)
 	rows, err = f.GetRows("Sheet1")
 	assert.Equal(t, [][]string{
 		{"A6", "B6", "C6"},
@@ -397,9 +400,6 @@ func TestGetCellValue(t *testing.T) {
 		{"", "", "", "", "", "", "", "H6"},
 	}, rows)
 	assert.NoError(t, err)
-	cell, err = f.GetCellValue("Sheet1", "H6")
-	assert.Equal(t, "H6", cell)
-	assert.NoError(t, err)
 
 	f.Sheet.Delete("xl/worksheets/sheet1.xml")
 	f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `<row><c r="A1" t="inlineStr"><is><t>A1</t></is></c></row><row></row><row><c r="A3" t="inlineStr"><is><t>A3</t></is></c></row>`)))

col.go

@@ -63,16 +63,16 @@ type Cols struct {
 //	    fmt.Println()
 //	}
 func (f *File) GetCols(sheet string, opts ...Options) ([][]string, error) {
-	if _, err := f.workSheetReader(sheet); err != nil {
+	cols, err := f.Cols(sheet)
+	if err != nil {
 		return nil, err
 	}
-	cols, err := f.Cols(sheet)
 	results := make([][]string, 0, 64)
 	for cols.Next() {
 		col, _ := cols.Rows(opts...)
 		results = append(results, col)
 	}
-	return results, err
+	return results, nil
 }
 
 // Next will return true if the next column is found.

col_test.go

@@ -140,7 +140,7 @@ func TestGetColsError(t *testing.T) {
 	f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(`<worksheet xmlns="%s"><sheetData><row r="A"><c r="2" t="inlineStr"><is><t>B</t></is></c></row></sheetData></worksheet>`, NameSpaceSpreadSheet.Value)))
 	f.checked = sync.Map{}
 	_, err = f.GetCols("Sheet1")
-	assert.EqualError(t, err, `strconv.ParseInt: parsing "A": invalid syntax`)
+	assert.EqualError(t, err, `strconv.Atoi: parsing "A": invalid syntax`)
 
 	f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(`<worksheet xmlns="%s"><sheetData><row r="2"><c r="A" t="inlineStr"><is><t>B</t></is></c></row></sheetData></worksheet>`, NameSpaceSpreadSheet.Value)))
 	_, err = f.GetCols("Sheet1")

excelize.go

@@ -305,84 +305,85 @@ func (f *File) workSheetReader(sheet string) (ws *xlsxWorksheet, err error) {
 // checkSheet provides a function to fill each row element and make that is
 // continuous in a worksheet of XML.
 func (ws *xlsxWorksheet) checkSheet() {
-	row, r0 := ws.checkSheetRows()
-	sheetData := xlsxSheetData{Row: make([]xlsxRow, row)}
-	row = 0
-	for _, r := range ws.SheetData.Row {
-		if r.R == nil {
-			row++
-			r.R = intPtr(row)
-			sheetData.Row[row-1] = r
-			continue
-		}
-		if *r.R == row && row > 0 {
-			sheetData.Row[*r.R-1].C = append(sheetData.Row[*r.R-1].C, r.C...)
-			continue
-		}
-		if *r.R != 0 {
-			sheetData.Row[*r.R-1] = r
-			row = *r.R
-		}
-	}
-	for i := 1; i <= len(sheetData.Row); i++ {
-		sheetData.Row[i-1].R = intPtr(i)
-	}
-	ws.checkSheetR0(&sheetData, r0)
-}
-
-// checkSheetRows returns the last row number of the worksheet and rows element
-// with r="0" attribute.
-func (ws *xlsxWorksheet) checkSheetRows() (int, []xlsxRow) {
 	var (
-		row, maxVal int
-		r0          []xlsxRow
-		maxRowNum   = func(num int, c []xlsxC) int {
-			for _, cell := range c {
-				if _, n, err := CellNameToCoordinates(cell.R); err == nil && n > num {
-					num = n
+		row        int
+		r0Rows     []xlsxRow
+		lastRowNum = func(r xlsxRow) int {
+			var num int
+			for _, cell := range r.C {
+				if _, row, err := CellNameToCoordinates(cell.R); err == nil {
+					if row > num {
+						num = row
+					}
 				}
 			}
 			return num
 		}
 	)
-	for i, r := range ws.SheetData.Row {
-		if r.R == nil {
-			row++
-			continue
-		}
-		if i == 0 && *r.R == 0 {
-			if num := maxRowNum(row, r.C); num > maxVal {
-				maxVal = num
+	for i := 0; i < len(ws.SheetData.Row); i++ {
+		r := ws.SheetData.Row[i]
+		if r.R == 0 || r.R == row {
+			num := lastRowNum(r)
+			if num > row {
+				row = num
 			}
-			r0 = append(r0, r)
+			if num == 0 {
+				row++
+			}
+			r.R = row
+			r0Rows = append(r0Rows, r)
+			ws.SheetData.Row = append(ws.SheetData.Row[:i], ws.SheetData.Row[i+1:]...)
+			i--
 			continue
 		}
-		if *r.R != 0 && *r.R > row {
-			row = *r.R
+		if r.R != 0 && r.R > row {
+			row = r.R
 		}
 	}
-	if maxVal > row {
-		row = maxVal
+	sheetData := xlsxSheetData{Row: make([]xlsxRow, row)}
+	row = 0
+	for _, r := range ws.SheetData.Row {
+		if r.R != 0 {
+			sheetData.Row[r.R-1] = r
+			row = r.R
+		}
+	}
+	for _, r0Row := range r0Rows {
+		sheetData.Row[r0Row.R-1].R = r0Row.R
+		ws.checkSheetR0(&sheetData, &r0Row, true)
+	}
+	for i := 1; i <= row; i++ {
+		sheetData.Row[i-1].R = i
+		ws.checkSheetR0(&sheetData, &sheetData.Row[i-1], false)
 	}
-	return row, r0
 }
 
 // checkSheetR0 handle the row element with r="0" attribute, cells in this row
 // could be disorderly, the cell in this row can be used as the value of
 // which cell is empty in the normal rows.
-func (ws *xlsxWorksheet) checkSheetR0(sheetData *xlsxSheetData, r0s []xlsxRow) {
-	for _, r0 := range r0s {
-		for _, cell := range r0.C {
-			if col, row, err := CellNameToCoordinates(cell.R); err == nil {
-				rowIdx := row - 1
-				columns, colIdx := len(sheetData.Row[rowIdx].C), col-1
-				for c := columns; c < col; c++ {
-					sheetData.Row[rowIdx].C = append(sheetData.Row[rowIdx].C, xlsxC{})
-				}
-				if !sheetData.Row[rowIdx].C[colIdx].hasValue() {
-					sheetData.Row[rowIdx].C[colIdx] = cell
-				}
-			}
+func (ws *xlsxWorksheet) checkSheetR0(sheetData *xlsxSheetData, rowData *xlsxRow, r0 bool) {
+	checkRow := func(col, row int, r0 bool, cell xlsxC) {
+		rowIdx := row - 1
+		columns, colIdx := len(sheetData.Row[rowIdx].C), col-1
+		for c := columns; c < col; c++ {
+			sheetData.Row[rowIdx].C = append(sheetData.Row[rowIdx].C, xlsxC{})
+		}
+		if !sheetData.Row[rowIdx].C[colIdx].hasValue() {
+			sheetData.Row[rowIdx].C[colIdx] = cell
+		}
+		if r0 {
+			sheetData.Row[rowIdx].C[colIdx] = cell
+		}
+	}
+	var err error
+	for i, cell := range rowData.C {
+		col, row := i+1, rowData.R
+		if cell.R == "" {
+			checkRow(col, row, r0, cell)
+			continue
+		}
+		if col, row, err = CellNameToCoordinates(cell.R); err == nil && r0 {
+			checkRow(col, row, r0, cell)
 		}
 	}
 	ws.SheetData = *sheetData

rows.go

@@ -59,10 +59,10 @@ var duplicateHelperFunc = [3]func(*File, *xlsxWorksheet, string, int, int) error
 //	    fmt.Println()
 //	}
 func (f *File) GetRows(sheet string, opts ...Options) ([][]string, error) {
-	if _, err := f.workSheetReader(sheet); err != nil {
+	rows, err := f.Rows(sheet)
+	if err != nil {
 		return nil, err
 	}
-	rows, _ := f.Rows(sheet)
 	results, cur, maxVal := make([][]string, 0, 64), 0, 0
 	for rows.Next() {
 		cur++
@@ -392,7 +392,7 @@ func (f *File) getRowHeight(sheet string, row int) int {
 	defer ws.mu.Unlock()
 	for i := range ws.SheetData.Row {
 		v := &ws.SheetData.Row[i]
-		if v.R != nil && *v.R == row && v.Ht != nil {
+		if v.R == row && v.Ht != nil {
 			return int(convertRowHeightToPixels(*v.Ht))
 		}
 	}
@@ -423,7 +423,7 @@ func (f *File) GetRowHeight(sheet string, row int) (float64, error) {
 		return ht, nil // it will be better to use 0, but we take care with BC
 	}
 	for _, v := range ws.SheetData.Row {
-		if v.R != nil && *v.R == row && v.Ht != nil {
+		if v.R == row && v.Ht != nil {
 			return *v.Ht, nil
 		}
 	}
@@ -578,7 +578,7 @@ func (f *File) RemoveRow(sheet string, row int) error {
 	keep := 0
 	for rowIdx := 0; rowIdx < len(ws.SheetData.Row); rowIdx++ {
 		v := &ws.SheetData.Row[rowIdx]
-		if v.R != nil && *v.R != row {
+		if v.R != row {
 			ws.SheetData.Row[keep] = *v
 			keep++
 		}
@@ -649,7 +649,7 @@ func (f *File) DuplicateRowTo(sheet string, row, row2 int) error {
 	var rowCopy xlsxRow
 
 	for i, r := range ws.SheetData.Row {
-		if *r.R == row {
+		if r.R == row {
 			rowCopy = deepcopy.Copy(ws.SheetData.Row[i]).(xlsxRow)
 			ok = true
 			break
@@ -666,7 +666,7 @@ func (f *File) DuplicateRowTo(sheet string, row, row2 int) error {
 
 	idx2 := -1
 	for i, r := range ws.SheetData.Row {
-		if *r.R == row2 {
+		if r.R == row2 {
 			idx2 = i
 			break
 		}

sheet.go

@@ -1957,7 +1957,7 @@ func (ws *xlsxWorksheet) prepareSheetXML(col int, row int) {
 	if rowCount < row {
 		// append missing rows
 		for rowIdx := rowCount; rowIdx < row; rowIdx++ {
-			ws.SheetData.Row = append(ws.SheetData.Row, xlsxRow{R: intPtr(rowIdx + 1), CustomHeight: customHeight, Ht: ht, C: make([]xlsxC, 0, sizeHint)})
+			ws.SheetData.Row = append(ws.SheetData.Row, xlsxRow{R: rowIdx + 1, CustomHeight: customHeight, Ht: ht, C: make([]xlsxC, 0, sizeHint)})
 		}
 	}
 	rowData := &ws.SheetData.Row[row-1]

xmlWorksheet.go

@@ -308,7 +308,7 @@ type xlsxSheetData struct {
 // particular row in the worksheet.
 type xlsxRow struct {
 	C            []xlsxC  `xml:"c"`
-	R            *int     `xml:"r,attr"`
+	R            int      `xml:"r,attr,omitempty"`
 	Spans        string   `xml:"spans,attr,omitempty"`
 	S            int      `xml:"s,attr,omitempty"`
 	CustomFormat bool     `xml:"customFormat,attr,omitempty"`