Skip to content

Commit 4eb088c

Browse files
ha5kyha5ky
authored
This fix performance impact introduced in #1692 (#1849)
Co-authored-by: chun.zhang2 <[email protected]> - This fix speed slowdown and memory usage increase base on the reverts commit 6220a79 - Fix panic on read workbook with internal row element without r attribute - Update the unit tests
1 parent 585ebff commit 4eb088c

10 files changed

+88
-87
lines changed

Diff for: ‎adjust.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -201,13 +201,13 @@ func (f *File) adjustRowDimensions(sheet string, ws *xlsxWorksheet, row, offset
201201
return nil
202202
}
203203
lastRow := &ws.SheetData.Row[totalRows-1]
204-
if newRow := *lastRow.R + offset; *lastRow.R >= row && newRow > 0 && newRow > TotalRows {
204+
if newRow := lastRow.R + offset; lastRow.R >= row && newRow > 0 && newRow > TotalRows {
205205
return ErrMaxRows
206206
}
207207
numOfRows := len(ws.SheetData.Row)
208208
for i := 0; i < numOfRows; i++ {
209209
r := &ws.SheetData.Row[i]
210-
if newRow := *r.R + offset; *r.R >= row && newRow > 0 {
210+
if newRow := r.R + offset; r.R >= row && newRow > 0 {
211211
r.adjustSingleRowDimensions(offset)
212212
}
213213
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
219219

220220
// adjustSingleRowDimensions provides a function to adjust single row dimensions.
221221
func (r *xlsxRow) adjustSingleRowDimensions(offset int) {
222-
r.R = intPtr(*r.R + offset)
222+
r.R += offset
223223
for i, col := range r.C {
224224
colName, _, _ := SplitCellName(col.R)
225-
r.C[i].R, _ = JoinCellName(colName, *r.R)
225+
r.C[i].R, _ = JoinCellName(colName, r.R)
226226
}
227227
}
228228

@@ -701,7 +701,7 @@ func (f *File) adjustAutoFilter(ws *xlsxWorksheet, sheet string, dir adjustDirec
701701
ws.AutoFilter = nil
702702
for rowIdx := range ws.SheetData.Row {
703703
rowData := &ws.SheetData.Row[rowIdx]
704-
if rowData.R != nil && *rowData.R > y1 && *rowData.R <= y2 {
704+
if rowData.R > y1 && rowData.R <= y2 {
705705
rowData.Hidden = false
706706
}
707707
}

Diff for: ‎adjust_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ func TestAdjustAutoFilter(t *testing.T) {
289289
f := NewFile()
290290
assert.NoError(t, f.adjustAutoFilter(&xlsxWorksheet{
291291
SheetData: xlsxSheetData{
292-
Row: []xlsxRow{{Hidden: true, R: intPtr(2)}},
292+
Row: []xlsxRow{{Hidden: true, R: 2}},
293293
},
294294
AutoFilter: &xlsxAutoFilter{
295295
Ref: "A1:A3",

Diff for: ‎cell.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -1426,8 +1426,8 @@ func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c
14261426
return "", err
14271427
}
14281428
lastRowNum := 0
1429-
if l := len(ws.SheetData.Row); l > 0 && ws.SheetData.Row[l-1].R != nil {
1430-
lastRowNum = *ws.SheetData.Row[l-1].R
1429+
if l := len(ws.SheetData.Row); l > 0 {
1430+
lastRowNum = ws.SheetData.Row[l-1].R
14311431
}
14321432

14331433
// keep in mind: row starts from 1
@@ -1437,7 +1437,7 @@ func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c
14371437

14381438
for rowIdx := range ws.SheetData.Row {
14391439
rowData := &ws.SheetData.Row[rowIdx]
1440-
if rowData.R != nil && *rowData.R != row {
1440+
if rowData.R != row {
14411441
continue
14421442
}
14431443
for colIdx := range rowData.C {

Diff for: ‎cell_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ func TestSetCellValue(t *testing.T) {
278278
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)))
279279
f.Sheet.Store("xl/worksheets/sheet1.xml", &xlsxWorksheet{
280280
SheetData: xlsxSheetData{Row: []xlsxRow{
281-
{R: intPtr(1), C: []xlsxC{{R: "A1", T: "str", V: "1"}}},
281+
{R: 1, C: []xlsxC{{R: "A1", T: "str", V: "1"}}},
282282
}},
283283
})
284284
assert.NoError(t, f.SetCellValue("Sheet1", "A1", "b"))
@@ -387,6 +387,9 @@ func TestGetCellValue(t *testing.T) {
387387
f.Sheet.Delete("xl/worksheets/sheet1.xml")
388388
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>`)))
389389
f.checked = sync.Map{}
390+
cell, err = f.GetCellValue("Sheet1", "H6")
391+
assert.Equal(t, "H6", cell)
392+
assert.NoError(t, err)
390393
rows, err = f.GetRows("Sheet1")
391394
assert.Equal(t, [][]string{
392395
{"A6", "B6", "C6"},
@@ -397,9 +400,6 @@ func TestGetCellValue(t *testing.T) {
397400
{"", "", "", "", "", "", "", "H6"},
398401
}, rows)
399402
assert.NoError(t, err)
400-
cell, err = f.GetCellValue("Sheet1", "H6")
401-
assert.Equal(t, "H6", cell)
402-
assert.NoError(t, err)
403403

404404
f.Sheet.Delete("xl/worksheets/sheet1.xml")
405405
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>`)))

Diff for: ‎col.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -63,16 +63,16 @@ type Cols struct {
6363
// fmt.Println()
6464
// }
6565
func (f *File) GetCols(sheet string, opts ...Options) ([][]string, error) {
66-
if _, err := f.workSheetReader(sheet); err != nil {
66+
cols, err := f.Cols(sheet)
67+
if err != nil {
6768
return nil, err
6869
}
69-
cols, err := f.Cols(sheet)
7070
results := make([][]string, 0, 64)
7171
for cols.Next() {
7272
col, _ := cols.Rows(opts...)
7373
results = append(results, col)
7474
}
75-
return results, err
75+
return results, nil
7676
}
7777

7878
// Next will return true if the next column is found.

Diff for: ‎col_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ func TestGetColsError(t *testing.T) {
140140
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)))
141141
f.checked = sync.Map{}
142142
_, err = f.GetCols("Sheet1")
143-
assert.EqualError(t, err, `strconv.ParseInt: parsing "A": invalid syntax`)
143+
assert.EqualError(t, err, `strconv.Atoi: parsing "A": invalid syntax`)
144144

145145
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)))
146146
_, err = f.GetCols("Sheet1")

Diff for: ‎excelize.go

+62-61
Original file line numberDiff line numberDiff line change
@@ -305,84 +305,85 @@ func (f *File) workSheetReader(sheet string) (ws *xlsxWorksheet, err error) {
305305
// checkSheet provides a function to fill each row element and make that is
306306
// continuous in a worksheet of XML.
307307
func (ws *xlsxWorksheet) checkSheet() {
308-
row, r0 := ws.checkSheetRows()
309-
sheetData := xlsxSheetData{Row: make([]xlsxRow, row)}
310-
row = 0
311-
for _, r := range ws.SheetData.Row {
312-
if r.R == nil {
313-
row++
314-
r.R = intPtr(row)
315-
sheetData.Row[row-1] = r
316-
continue
317-
}
318-
if *r.R == row && row > 0 {
319-
sheetData.Row[*r.R-1].C = append(sheetData.Row[*r.R-1].C, r.C...)
320-
continue
321-
}
322-
if *r.R != 0 {
323-
sheetData.Row[*r.R-1] = r
324-
row = *r.R
325-
}
326-
}
327-
for i := 1; i <= len(sheetData.Row); i++ {
328-
sheetData.Row[i-1].R = intPtr(i)
329-
}
330-
ws.checkSheetR0(&sheetData, r0)
331-
}
332-
333-
// checkSheetRows returns the last row number of the worksheet and rows element
334-
// with r="0" attribute.
335-
func (ws *xlsxWorksheet) checkSheetRows() (int, []xlsxRow) {
336308
var (
337-
row, maxVal int
338-
r0 []xlsxRow
339-
maxRowNum = func(num int, c []xlsxC) int {
340-
for _, cell := range c {
341-
if _, n, err := CellNameToCoordinates(cell.R); err == nil && n > num {
342-
num = n
309+
row int
310+
r0Rows []xlsxRow
311+
lastRowNum = func(r xlsxRow) int {
312+
var num int
313+
for _, cell := range r.C {
314+
if _, row, err := CellNameToCoordinates(cell.R); err == nil {
315+
if row > num {
316+
num = row
317+
}
343318
}
344319
}
345320
return num
346321
}
347322
)
348-
for i, r := range ws.SheetData.Row {
349-
if r.R == nil {
350-
row++
351-
continue
352-
}
353-
if i == 0 && *r.R == 0 {
354-
if num := maxRowNum(row, r.C); num > maxVal {
355-
maxVal = num
323+
for i := 0; i < len(ws.SheetData.Row); i++ {
324+
r := ws.SheetData.Row[i]
325+
if r.R == 0 || r.R == row {
326+
num := lastRowNum(r)
327+
if num > row {
328+
row = num
356329
}
357-
r0 = append(r0, r)
330+
if num == 0 {
331+
row++
332+
}
333+
r.R = row
334+
r0Rows = append(r0Rows, r)
335+
ws.SheetData.Row = append(ws.SheetData.Row[:i], ws.SheetData.Row[i+1:]...)
336+
i--
358337
continue
359338
}
360-
if *r.R != 0 && *r.R > row {
361-
row = *r.R
339+
if r.R != 0 && r.R > row {
340+
row = r.R
362341
}
363342
}
364-
if maxVal > row {
365-
row = maxVal
343+
sheetData := xlsxSheetData{Row: make([]xlsxRow, row)}
344+
row = 0
345+
for _, r := range ws.SheetData.Row {
346+
if r.R != 0 {
347+
sheetData.Row[r.R-1] = r
348+
row = r.R
349+
}
350+
}
351+
for _, r0Row := range r0Rows {
352+
sheetData.Row[r0Row.R-1].R = r0Row.R
353+
ws.checkSheetR0(&sheetData, &r0Row, true)
354+
}
355+
for i := 1; i <= row; i++ {
356+
sheetData.Row[i-1].R = i
357+
ws.checkSheetR0(&sheetData, &sheetData.Row[i-1], false)
366358
}
367-
return row, r0
368359
}
369360

370361
// checkSheetR0 handle the row element with r="0" attribute, cells in this row
371362
// could be disorderly, the cell in this row can be used as the value of
372363
// which cell is empty in the normal rows.
373-
func (ws *xlsxWorksheet) checkSheetR0(sheetData *xlsxSheetData, r0s []xlsxRow) {
374-
for _, r0 := range r0s {
375-
for _, cell := range r0.C {
376-
if col, row, err := CellNameToCoordinates(cell.R); err == nil {
377-
rowIdx := row - 1
378-
columns, colIdx := len(sheetData.Row[rowIdx].C), col-1
379-
for c := columns; c < col; c++ {
380-
sheetData.Row[rowIdx].C = append(sheetData.Row[rowIdx].C, xlsxC{})
381-
}
382-
if !sheetData.Row[rowIdx].C[colIdx].hasValue() {
383-
sheetData.Row[rowIdx].C[colIdx] = cell
384-
}
385-
}
364+
func (ws *xlsxWorksheet) checkSheetR0(sheetData *xlsxSheetData, rowData *xlsxRow, r0 bool) {
365+
checkRow := func(col, row int, r0 bool, cell xlsxC) {
366+
rowIdx := row - 1
367+
columns, colIdx := len(sheetData.Row[rowIdx].C), col-1
368+
for c := columns; c < col; c++ {
369+
sheetData.Row[rowIdx].C = append(sheetData.Row[rowIdx].C, xlsxC{})
370+
}
371+
if !sheetData.Row[rowIdx].C[colIdx].hasValue() {
372+
sheetData.Row[rowIdx].C[colIdx] = cell
373+
}
374+
if r0 {
375+
sheetData.Row[rowIdx].C[colIdx] = cell
376+
}
377+
}
378+
var err error
379+
for i, cell := range rowData.C {
380+
col, row := i+1, rowData.R
381+
if cell.R == "" {
382+
checkRow(col, row, r0, cell)
383+
continue
384+
}
385+
if col, row, err = CellNameToCoordinates(cell.R); err == nil && r0 {
386+
checkRow(col, row, r0, cell)
386387
}
387388
}
388389
ws.SheetData = *sheetData

Diff for: ‎rows.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,10 @@ var duplicateHelperFunc = [3]func(*File, *xlsxWorksheet, string, int, int) error
5959
// fmt.Println()
6060
// }
6161
func (f *File) GetRows(sheet string, opts ...Options) ([][]string, error) {
62-
if _, err := f.workSheetReader(sheet); err != nil {
62+
rows, err := f.Rows(sheet)
63+
if err != nil {
6364
return nil, err
6465
}
65-
rows, _ := f.Rows(sheet)
6666
results, cur, maxVal := make([][]string, 0, 64), 0, 0
6767
for rows.Next() {
6868
cur++
@@ -392,7 +392,7 @@ func (f *File) getRowHeight(sheet string, row int) int {
392392
defer ws.mu.Unlock()
393393
for i := range ws.SheetData.Row {
394394
v := &ws.SheetData.Row[i]
395-
if v.R != nil && *v.R == row && v.Ht != nil {
395+
if v.R == row && v.Ht != nil {
396396
return int(convertRowHeightToPixels(*v.Ht))
397397
}
398398
}
@@ -423,7 +423,7 @@ func (f *File) GetRowHeight(sheet string, row int) (float64, error) {
423423
return ht, nil // it will be better to use 0, but we take care with BC
424424
}
425425
for _, v := range ws.SheetData.Row {
426-
if v.R != nil && *v.R == row && v.Ht != nil {
426+
if v.R == row && v.Ht != nil {
427427
return *v.Ht, nil
428428
}
429429
}
@@ -578,7 +578,7 @@ func (f *File) RemoveRow(sheet string, row int) error {
578578
keep := 0
579579
for rowIdx := 0; rowIdx < len(ws.SheetData.Row); rowIdx++ {
580580
v := &ws.SheetData.Row[rowIdx]
581-
if v.R != nil && *v.R != row {
581+
if v.R != row {
582582
ws.SheetData.Row[keep] = *v
583583
keep++
584584
}
@@ -649,7 +649,7 @@ func (f *File) DuplicateRowTo(sheet string, row, row2 int) error {
649649
var rowCopy xlsxRow
650650

651651
for i, r := range ws.SheetData.Row {
652-
if *r.R == row {
652+
if r.R == row {
653653
rowCopy = deepcopy.Copy(ws.SheetData.Row[i]).(xlsxRow)
654654
ok = true
655655
break
@@ -666,7 +666,7 @@ func (f *File) DuplicateRowTo(sheet string, row, row2 int) error {
666666

667667
idx2 := -1
668668
for i, r := range ws.SheetData.Row {
669-
if *r.R == row2 {
669+
if r.R == row2 {
670670
idx2 = i
671671
break
672672
}

Diff for: ‎sheet.go

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

Diff for: ‎xmlWorksheet.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ type xlsxSheetData struct {
308308
// particular row in the worksheet.
309309
type xlsxRow struct {
310310
C []xlsxC `xml:"c"`
311-
R *int `xml:"r,attr"`
311+
R int `xml:"r,attr,omitempty"`
312312
Spans string `xml:"spans,attr,omitempty"`
313313
S int `xml:"s,attr,omitempty"`
314314
CustomFormat bool `xml:"customFormat,attr,omitempty"`

0 commit comments

Comments
 (0)