perf: better caching for the parseExports function

gabotechs · Dec 25, 2023 · 7ad94f8 · 7ad94f8
1 parent 29e8553
commit 7ad94f8
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 31 deletions.
diff --git a/internal/language/exports.go b/internal/language/exports.go
@@ -53,22 +53,6 @@ func (p *Parser[F]) parseExports(
 	ctx context.Context,
 	id string,
 	unwrappedExports bool,
-) (context.Context, *ExportsResult, error) {
-	unwrappedCacheKey := parseExportsKey(fmt.Sprintf("%s-%t", id, unwrappedExports))
-	if cached, ok := ctx.Value(unwrappedCacheKey).(*ExportsResult); ok {
-		return ctx, cached, nil
-	}
-	ctx, result, err := p.uncachedParseExports(ctx, id, unwrappedExports, nil)
-	if err != nil {
-		return ctx, nil, err
-	}
-	return context.WithValue(ctx, unwrappedCacheKey, result), result, nil
-}
-
-func (p *Parser[F]) uncachedParseExports(
-	ctx context.Context,
-	id string,
-	unwrappedExports bool,
 	stack *utils.CallStack,
 ) (context.Context, *ExportsResult, error) {
 	if stack == nil {
@@ -77,6 +61,11 @@ func (p *Parser[F]) uncachedParseExports(
 	if err := stack.Push(id); err != nil {
 		return ctx, nil, errors.New("circular export: " + err.Error())
 	}
+	defer stack.Pop()
+	unwrappedCacheKey := parseExportsKey(fmt.Sprintf("%s-%t", stack.Hash(), unwrappedExports))
+	if cached, ok := ctx.Value(unwrappedCacheKey).(*ExportsResult); ok {
+		return ctx, cached, nil
+	}
 
 	ctx, wrapped, err := p.gatherExportsFromFile(ctx, id)
 	if err != nil {
@@ -94,7 +83,7 @@ func (p *Parser[F]) uncachedParseExports(
 		}
 
 		var unwrapped *ExportsResult
-		ctx, unwrapped, err = p.uncachedParseExports(ctx, export.Path, unwrappedExports, stack)
+		ctx, unwrapped, err = p.parseExports(ctx, export.Path, unwrappedExports, stack)
 		if err != nil {
 			exportErrors = append(exportErrors, err)
 			continue
@@ -126,8 +115,8 @@ func (p *Parser[F]) uncachedParseExports(
 		}
 	}
 
-	stack.Pop()
-	return ctx, &ExportsResult{Exports: exports, Errors: exportErrors}, nil
+	result := ExportsResult{Exports: exports, Errors: exportErrors}
+	return context.WithValue(ctx, unwrappedCacheKey, &result), &result, nil
 }
 
 type gatherExportsFromFileKey string

diff --git a/internal/language/exports_test.go b/internal/language/exports_test.go
@@ -218,6 +218,50 @@ func TestParser_CachedUnwrappedParseExports(t *testing.T) {
 				"circular export: cycle detected:\n1\n2\n3\n4\n1",
 			},
 		},
+		{
+			Name: "non circular export",
+			Path: "1",
+			Exports: b().
+				Entry("1", "2", "A").
+				Entry("1", "3", "A").
+				Entry("1", "4", "A").
+				Entry("2", "2", "A").
+				Entry("3", "2", "A").
+				Entry("4", "3", "A").
+				Build(),
+			ExpectedUnwrapped: makeStringOm(
+				"A", "2",
+			),
+			ExpectedWrapped: makeStringOm(
+				"A", "2",
+				"A", "3",
+				"A", "4",
+			),
+			ExpectedErrors: []string{},
+		},
+		{
+			Name: "non circular export (2)",
+			Path: "1",
+			Exports: b().
+				Entry("1", "2", "A").
+				Entry("1", "2", "B").
+				Entry("1", "2", "C").
+				Entry("2", "2", "A").
+				Entry("2", "2", "B").
+				Entry("2", "2", "C").
+				Build(),
+			ExpectedUnwrapped: makeStringOm(
+				"A", "2",
+				"B", "2",
+				"C", "2",
+			),
+			ExpectedWrapped: makeStringOm(
+				"A", "2",
+				"B", "2",
+				"C", "2",
+			),
+			ExpectedErrors: []string{},
+		},
 	}
 
 	for _, tt := range tests {
@@ -228,21 +272,25 @@ func TestParser_CachedUnwrappedParseExports(t *testing.T) {
 				exports: tt.Exports,
 			}
 			parser := lang.testParser(tt.Path)
+			ctx := context.Background()
 
-			_, exports, err := parser.parseExports(context.Background(), "1", true)
-			a.NoError(err)
+			for i := 0; i < 2; i++ {
+				nCtx, exports, err := parser.parseExports(ctx, "1", true, nil)
+				ctx = nCtx
+				a.NoError(err)
 
-			a.Equal(tt.ExpectedUnwrapped, exports.Exports)
+				a.Equal(tt.ExpectedUnwrapped, exports.Exports)
 
-			_, exports, err = parser.parseExports(context.Background(), "1", false)
-			a.NoError(err)
+				ctx, exports, err = parser.parseExports(ctx, "1", false, nil)
+				a.NoError(err)
 
-			a.Equal(tt.ExpectedWrapped, exports.Exports)
-			var expectedErrors []error
-			for _, expectedError := range tt.ExpectedErrors {
-				expectedErrors = append(expectedErrors, errors.New(expectedError))
+				a.Equal(tt.ExpectedWrapped, exports.Exports)
+				var expectedErrors []error
+				for _, expectedError := range tt.ExpectedErrors {
+					expectedErrors = append(expectedErrors, errors.New(expectedError))
+				}
+				a.Equal(expectedErrors, exports.Errors)
 			}
-			a.Equal(expectedErrors, exports.Errors)
 		})
 	}
 }
diff --git a/internal/language/parser.go b/internal/language/parser.go
@@ -136,7 +136,7 @@ func (p *Parser[F]) Deps(ctx context.Context, n *Node) (context.Context, []*Node
 		//  technically is true, but it's not true to say that `foo` is imported from `bar.ts`.
 		//  It's more accurate to say that `bar` is imported from `bar.ts`, even if the alias is `foo`.
 		//  Instead we never unwrap export to avoid this.
-		ctx, exports, err = p.parseExports(ctx, n.Id, false)
+		ctx, exports, err = p.parseExports(ctx, n.Id, false, nil)
 		if err != nil {
 			return nil, nil, err
 		}
@@ -161,7 +161,7 @@ func (p *Parser[F]) Deps(ctx context.Context, n *Node) (context.Context, []*Node
 		}
 
 		var exports *ExportsResult
-		ctx, exports, err = p.parseExports(ctx, importEntry.Path, true)
+		ctx, exports, err = p.parseExports(ctx, importEntry.Path, true, nil)
 		if err != nil {
 			return ctx, nil, err
 		}

diff --git a/internal/utils/callstack.go b/internal/utils/callstack.go
@@ -2,6 +2,7 @@ package utils
 
 import (
 	"errors"
+	"strings"
 
 	"github.com/elliotchance/orderedmap/v2"
 )
@@ -50,3 +51,7 @@ func (cs *CallStack) Back() (string, bool) {
 func (cs *CallStack) Stack() []string {
 	return cs.m.Keys()
 }
+
+func (cs *CallStack) Hash() string {
+	return strings.Join(cs.m.Keys(), "-")
+}