Merge pull request #827 from tealeg/speed-up-sharedstringreftable

Stop allocating in the loop when we load the Shared Strings Ref Table
tealeg · Nov 1, 2024 · 7c33d07 · 7c33d07
2 parents cbf4534 + 9d67881
commit 7c33d07
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 34 deletions.
diff --git a/file.go b/file.go
@@ -202,15 +202,15 @@ func (f *File) AddSheetWithCellStore(sheetName string, constructor CellStoreCons
 	if _, exists := f.Sheet[sheetName]; exists {
 		return nil, fmt.Errorf("duplicate sheet name '%s'.", sheetName)
 	}
-	
+
 	if err := IsSaneSheetName(sheetName); err != nil {
 		return nil, fmt.Errorf("sheet name is not valid: %w", err)
 	}
 	sheet := &Sheet{
-		Name:     sheetName,
-		File:     f,
-		Selected: len(f.Sheets) == 0,
-		Cols:     &ColStore{},
+		Name:          sheetName,
+		File:          f,
+		Selected:      len(f.Sheets) == 0,
+		Cols:          &ColStore{},
 		cellStoreName: sheetName,
 	}
 
@@ -335,7 +335,7 @@ func autoFilterDefinedName(sheet *Sheet, sheetIndex int) (*xlsxDefinedName, erro
 // representing the file in terms of the structure of an XLSX file.
 func (f *File) MakeStreamParts() (map[string]string, error) {
 	var parts map[string]string
-	var refTable *RefTable = NewSharedStringRefTable()
+	var refTable *RefTable = NewSharedStringRefTable(10000) // 10000 is arbitrary
 	refTable.isWrite = true
 	var workbookRels WorkBookRels = make(WorkBookRels)
 	var err error
@@ -465,7 +465,7 @@ func (f *File) MakeStreamParts() (map[string]string, error) {
 // MarshallParts constructs a map of file name to XML content representing the file
 // in terms of the structure of an XLSX file.
 func (f *File) MarshallParts(zipWriter *zip.Writer) error {
-	var refTable *RefTable = NewSharedStringRefTable()
+	var refTable *RefTable = NewSharedStringRefTable(10000) // 10000 is arbitrary
 	refTable.isWrite = true
 	var workbookRels WorkBookRels = make(WorkBookRels)
 	var err error

diff --git a/file_test.go b/file_test.go
@@ -405,7 +405,6 @@ func TestFile(t *testing.T) {
 
 		blokes, err := OpenFile(p)
 		c.Assert(err, qt.IsNil)
-
 
 		dave := blokes.Sheets[0]
 		if dave.currentRow != nil {

diff --git a/memory.go b/memory.go
@@ -102,9 +102,10 @@ func (mr *MemoryRow) ForEachCell(cvf CellVisitorFunc, option ...CellVisitorOptio
 		}
 	}
 	cellCount := len(mr.cells)
+	var c *Cell
 	if !flags.skipEmptyCells {
 		for ci := cellCount; ci < mr.row.Sheet.MaxCol; ci++ {
-			c := mr.GetCell(ci)
+			c = mr.GetCell(ci)
 			err := cvf(c)
 			if err != nil {
 				return err

diff --git a/reftable.go b/reftable.go
@@ -14,10 +14,11 @@ type RefTable struct {
 }
 
 // NewSharedStringRefTable creates a new, empty RefTable.
-func NewSharedStringRefTable() *RefTable {
+func NewSharedStringRefTable(size int) *RefTable {
 	rt := RefTable{}
-	rt.knownStrings = make(map[string]int)
-	rt.knownRichTexts = make(map[string][]int)
+	rt.indexedStrings = make([]plainTextOrRichText, 0, size)
+	rt.knownStrings = make(map[string]int, size)
+	rt.knownRichTexts = make(map[string][]int, size)
 	return &rt
 }
 
@@ -26,7 +27,7 @@ func NewSharedStringRefTable() *RefTable {
 // by numeric index - this is the model used within XLSX worksheet (a
 // numeric reference is stored to a shared cell value).
 func MakeSharedStringRefTable(source *xlsxSST) *RefTable {
-	reftable := NewSharedStringRefTable()
+	reftable := NewSharedStringRefTable(len(source.SI))
 	reftable.isWrite = false
 	for _, si := range source.SI {
 		if len(si.R) > 0 {

diff --git a/reftable_test.go b/reftable_test.go
@@ -50,7 +50,7 @@ var reftabletest_sharedStringsXMLStr = (`<?xml version="1.0" encoding="UTF-8" st
 // We can add a new string to the RefTable
 func TestRefTableAddString(t *testing.T) {
 	c := qt.New(t)
-	refTable := NewSharedStringRefTable()
+	refTable := NewSharedStringRefTable(1)
 	index := refTable.AddString("Foo")
 	c.Assert(index, qt.Equals, 0)
 	p, r := refTable.ResolveSharedString(0)
@@ -60,7 +60,7 @@ func TestRefTableAddString(t *testing.T) {
 
 func TestCreateNewSharedStringRefTable(t *testing.T) {
 	c := qt.New(t)
-	refTable := NewSharedStringRefTable()
+	refTable := NewSharedStringRefTable(2)
 	refTable.AddString("Foo")
 	refTable.AddString("Bar")
 	p, r := refTable.ResolveSharedString(0)
@@ -119,7 +119,7 @@ func TestResolveSharedString(t *testing.T) {
 // Test we can correctly create the xlsx.xlsxSST struct from a RefTable
 func TestMakeXLSXSST(t *testing.T) {
 	c := qt.New(t)
-	refTable := NewSharedStringRefTable()
+	refTable := NewSharedStringRefTable(2)
 	refTable.AddString("Foo")
 	refTable.AddString("Bar")
 	refTable.AddRichText([]RichTextRun{
@@ -154,7 +154,7 @@ func TestMakeXLSXSST(t *testing.T) {
 
 func TestMarshalSST(t *testing.T) {
 	c := qt.New(t)
-	refTable := NewSharedStringRefTable()
+	refTable := NewSharedStringRefTable(1)
 	refTable.AddString("Foo")
 	refTable.AddRichText([]RichTextRun{
 		{
@@ -185,7 +185,7 @@ func TestMarshalSST(t *testing.T) {
 
 func TestRefTableReadAddString(t *testing.T) {
 	c := qt.New(t)
-	refTable := NewSharedStringRefTable()
+	refTable := NewSharedStringRefTable(2)
 	refTable.isWrite = false
 	index1 := refTable.AddString("Foo")
 	index2 := refTable.AddString("Foo")
@@ -201,7 +201,7 @@ func TestRefTableReadAddString(t *testing.T) {
 
 func TestRefTableWriteAddString(t *testing.T) {
 	c := qt.New(t)
-	refTable := NewSharedStringRefTable()
+	refTable := NewSharedStringRefTable(2)
 	refTable.isWrite = true
 	index1 := refTable.AddString("Foo")
 	index2 := refTable.AddString("Foo")
@@ -214,7 +214,7 @@ func TestRefTableWriteAddString(t *testing.T) {
 
 func TestRefTableReadAddRichText(t *testing.T) {
 	c := qt.New(t)
-	refTable := NewSharedStringRefTable()
+	refTable := NewSharedStringRefTable(2)
 	refTable.isWrite = false
 	index1 := refTable.AddRichText([]RichTextRun{
 		{
@@ -253,7 +253,7 @@ func TestRefTableReadAddRichText(t *testing.T) {
 
 func TestRefTableWriteAddRichText(t *testing.T) {
 	c := qt.New(t)
-	refTable := NewSharedStringRefTable()
+	refTable := NewSharedStringRefTable(2)
 	refTable.isWrite = true
 	index1 := refTable.AddRichText([]RichTextRun{
 		{

diff --git a/sheet_test.go b/sheet_test.go
@@ -186,7 +186,7 @@ func TestSheet(t *testing.T) {
 
 		var buf bytes.Buffer
 
-		refTable := NewSharedStringRefTable()
+		refTable := NewSharedStringRefTable(1)
 		styles := newXlsxStyleSheet(nil)
 		err := sheet.MarshalSheet(&buf, refTable, styles, nil)
 		c.Assert(err, qt.Equals, nil)
@@ -220,7 +220,7 @@ func TestSheet(t *testing.T) {
 		row := sheet.AddRow()
 		cell := row.AddCell()
 		cell.Value = "A cell!"
-		refTable := NewSharedStringRefTable()
+		refTable := NewSharedStringRefTable(1)
 		styles := newXlsxStyleSheet(nil)
 		var output bytes.Buffer
 		err := sheet.MarshalSheet(&output, refTable, styles, nil)
@@ -256,7 +256,7 @@ func TestSheet(t *testing.T) {
 		cell1 := row.AddCell()
 		cell1.Value = "A cell!"
 
-		refTable := NewSharedStringRefTable()
+		refTable := NewSharedStringRefTable(1)
 		styles := newXlsxStyleSheet(nil)
 
 		var output bytes.Buffer
@@ -285,7 +285,7 @@ func TestSheet(t *testing.T) {
 
 		var buf bytes.Buffer
 
-		refTable := NewSharedStringRefTable()
+		refTable := NewSharedStringRefTable(1)
 		styles := newXlsxStyleSheet(nil)
 		err := sheet.MarshalSheet(&buf, refTable, styles, nil)
 		c.Assert(err, qt.Equals, nil)
@@ -303,7 +303,7 @@ func TestSheet(t *testing.T) {
 		row := sheet.AddRow()
 		cell := row.AddCell()
 		cell.Value = "A cell!"
-		refTable := NewSharedStringRefTable()
+		refTable := NewSharedStringRefTable(1)
 		styles := newXlsxStyleSheet(nil)
 		var output strings.Builder
 		err := sheet.MarshalSheet(&output, refTable, styles, nil)
@@ -325,7 +325,7 @@ func TestSheet(t *testing.T) {
 		cell.Value = "A cell (with value 2)!"
 		var buf bytes.Buffer
 
-		refTable := NewSharedStringRefTable()
+		refTable := NewSharedStringRefTable(2)
 		styles := newXlsxStyleSheet(nil)
 		err := sheet.MarshalSheet(&buf, refTable, styles, nil)
 		c.Assert(err, qt.Equals, nil)
@@ -457,7 +457,7 @@ func TestSheet(t *testing.T) {
 
 		var buf bytes.Buffer
 
-		refTable := NewSharedStringRefTable()
+		refTable := NewSharedStringRefTable(10)
 		styles := newXlsxStyleSheet(nil)
 		err := sheet.MarshalSheet(&buf, refTable, styles, nil)
 		c.Assert(err, qt.Equals, nil)
@@ -498,7 +498,7 @@ func TestMakeXLSXSheet(t *testing.T) {
 
 		var buf bytes.Buffer
 
-		refTable := NewSharedStringRefTable()
+		refTable := NewSharedStringRefTable(4)
 		styles := newXlsxStyleSheet(nil)
 		err := sheet.MarshalSheet(&buf, refTable, styles, nil)
 		c.Assert(err, qt.Equals, nil)
@@ -548,7 +548,7 @@ func TestMakeXLSXSheet(t *testing.T) {
 		cell2.SetStyle(style2)
 		var buf bytes.Buffer
 
-		refTable := NewSharedStringRefTable()
+		refTable := NewSharedStringRefTable(2)
 		styles := newXlsxStyleSheet(nil)
 		err := sheet.MarshalSheet(&buf, refTable, styles, nil)
 		c.Assert(err, qt.Equals, nil)
@@ -594,7 +594,7 @@ func TestMakeXLSXSheet(t *testing.T) {
 		sheet.SetColWidth(1, 1, 10.5)
 		var buf bytes.Buffer
 
-		refTable := NewSharedStringRefTable()
+		refTable := NewSharedStringRefTable(1)
 		styles := newXlsxStyleSheet(nil)
 		err := sheet.MarshalSheet(&buf, refTable, styles, nil)
 		c.Assert(err, qt.Equals, nil)
@@ -668,7 +668,7 @@ func TestMakeXLSXSheet(t *testing.T) {
 		cell1.SetStyle(style1)
 		var buf bytes.Buffer
 
-		refTable := NewSharedStringRefTable()
+		refTable := NewSharedStringRefTable(1)
 		styles := newXlsxStyleSheet(nil)
 		err := sheet.MarshalSheet(&buf, refTable, styles, nil)
 		c.Assert(err, qt.Equals, nil)
@@ -716,7 +716,7 @@ func TestMakeXLSXSheet(t *testing.T) {
 
 		// var buf bytes.Buffer
 
-		refTable := NewSharedStringRefTable()
+		refTable := NewSharedStringRefTable(9)
 		styles := newXlsxStyleSheet(nil)
 
 		xSheet := sheet.makeXLSXSheet(refTable, styles, nil)
@@ -796,7 +796,7 @@ func TestTemp(t *testing.T) {
 
 	var buf bytes.Buffer
 
-	refTable := NewSharedStringRefTable()
+	refTable := NewSharedStringRefTable(1)
 	styles := newXlsxStyleSheet(nil)
 	err := sheet.MarshalSheet(&buf, refTable, styles, nil)
 	c.Assert(err, qt.Equals, nil)