From 4eb088cf736120228ba69fd740f940c3b309cc86 Mon Sep 17 00:00:00 2001 From: hu5ky <1650546312@qq.com> Date: Fri, 15 Mar 2024 11:36:34 +0800 Subject: [PATCH] This fix performance impact introduced in #1692 (#1849) Co-authored-by: chun.zhang2 - This fix speed slowdown and memory usage increase base on the reverts commit 6220a798fd79231bf4b3d7ef587bb2b79d276710 - Fix panic on read workbook with internal row element without r attribute - Update the unit tests --- adjust.go | 10 ++-- adjust_test.go | 2 +- cell.go | 6 +-- cell_test.go | 8 ++-- col.go | 6 +-- col_test.go | 2 +- excelize.go | 123 ++++++++++++++++++++++++------------------------ rows.go | 14 +++--- sheet.go | 2 +- xmlWorksheet.go | 2 +- 10 files changed, 88 insertions(+), 87 deletions(-) diff --git a/adjust.go b/adjust.go index a92944d..46b8215 100644 --- a/adjust.go +++ b/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 } } diff --git a/adjust_test.go b/adjust_test.go index a6ea323..00b5112 100644 --- a/adjust_test.go +++ b/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", diff --git a/cell.go b/cell.go index 3f1dd48..e9b5730 100644 --- a/cell.go +++ b/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 { diff --git a/cell_test.go b/cell_test.go index 61178c2..7517d28 100644 --- a/cell_test.go +++ b/cell_test.go @@ -278,7 +278,7 @@ func TestSetCellValue(t *testing.T) { f.Pkg.Store(defaultXMLPathSharedStrings, []byte(fmt.Sprintf(`aa`, 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, `H6r0A6F4A6B6C6100B3`))) 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, `A1A3`))) diff --git a/col.go b/col.go index b1b9c0d..68ffc70 100644 --- a/col.go +++ b/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. diff --git a/col_test.go b/col_test.go index 6174254..2e7aeb8 100644 --- a/col_test.go +++ b/col_test.go @@ -140,7 +140,7 @@ func TestGetColsError(t *testing.T) { f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(`B`, 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(`B`, NameSpaceSpreadSheet.Value))) _, err = f.GetCols("Sheet1") diff --git a/excelize.go b/excelize.go index 87ef22d..de385de 100644 --- a/excelize.go +++ b/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 diff --git a/rows.go b/rows.go index 814ff9c..7610836 100644 --- a/rows.go +++ b/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 } diff --git a/sheet.go b/sheet.go index e089c41..f1267f7 100644 --- a/sheet.go +++ b/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] diff --git a/xmlWorksheet.go b/xmlWorksheet.go index c5e06b8..3a71d69 100644 --- a/xmlWorksheet.go +++ b/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"`