feat: Add profile to pixlet check. (#811)

This change adds profiling to the pixlet check command. In addition, it adds a render check given it needs to be able to render before we can profile an app. Finally, I've made improvements to output silencing so the pixlet check output is clear.
tidbyt · Jun 7, 2023 · e00ee60 · e00ee60
1 parent ccf92b1
commit e00ee60
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 42 deletions.
diff --git a/cmd/check.go b/cmd/check.go
@@ -5,6 +5,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/bazelbuild/buildtools/buildifier/utils"
 	"github.com/fatih/color"
@@ -13,6 +14,8 @@ import (
 	"tidbyt.dev/pixlet/manifest"
 )
 
+const MaxRenderTime = 500000000 // 500ms
+
 func init() {
 	CheckCmd.Flags().BoolVarP(&rflag, "recursive", "r", false, "find apps recursively")
 }
@@ -60,16 +63,6 @@ func checkCmd(cmd *cobra.Command, args []string) error {
 			continue
 		}
 
-		// Create temporary file for app rendering.
-		f, err := os.CreateTemp("", "")
-		if err != nil {
-			return fmt.Errorf("could not create temp file for rendering, check your system: %w", err)
-		}
-		defer os.Remove(f.Name())
-
-		// TODO: Check if app will render once we are able to enable target
-		// determination.
-
 		// Check if an app can load.
 		err = community.LoadApp(cmd, []string{app})
 		if err != nil {
@@ -121,8 +114,34 @@ func checkCmd(cmd *cobra.Command, args []string) error {
 			failure(app, fmt.Errorf("manifest contains spelling errors: %w", err), fmt.Sprintf("try `pixlet community spell-check --fix %s`", manifestFile))
 			continue
 		}
-		// TODO: enable spell check for apps once we can run it successfully
-		// against the community repo.
+
+		// Create temporary file for app rendering.
+		f, err := os.CreateTemp("", "")
+		if err != nil {
+			return fmt.Errorf("could not create temp file for rendering, check your system: %w", err)
+		}
+		defer os.Remove(f.Name())
+
+		// Check if app renders.
+		silenceOutput = true
+		output = f.Name()
+		err = render(cmd, []string{app})
+		if err != nil {
+			foundIssue = true
+			failure(app, fmt.Errorf("app failed to render: %w", err), "try `pixlet render` and resolve any runtime issues")
+			continue
+		}
+
+		// Check performance.
+		p, err := ProfileApp(app, map[string]string{})
+		if err != nil {
+			return fmt.Errorf("could not profile app: %w", err)
+		}
+		if p.DurationNanos > MaxRenderTime {
+			foundIssue = true
+			failure(app, fmt.Errorf("app takes too long to render %s", time.Duration(p.DurationNanos)), fmt.Sprintf("try optimizing your app using `pixlet profile %s` to get it under %s", app, time.Duration(MaxRenderTime)))
+			continue
+		}
 
 		// If we're here, the app and manifest are good to go!
 		success(app)

diff --git a/cmd/community/loadapp.go b/cmd/community/loadapp.go
@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/spf13/cobra"
+	"go.starlark.net/starlark"
 	"tidbyt.dev/pixlet/runtime"
 )
 
@@ -34,8 +35,15 @@ func LoadApp(cmd *cobra.Command, args []string) error {
 	runtime.InitHTTP(cache)
 	runtime.InitCache(cache)
 
+	// Remove the print function from the starlark thread.
+	initializers := []runtime.ThreadInitializer{}
+	initializers = append(initializers, func(thread *starlark.Thread) *starlark.Thread {
+		thread.Print = func(thread *starlark.Thread, msg string) {}
+		return thread
+	})
+
 	applet := runtime.Applet{}
-	err = applet.Load(script, src, nil)
+	err = applet.LoadWithInitializers(script, src, nil, initializers...)
 	if err != nil {
 		return fmt.Errorf("failed to load applet: %w", err)
 	}

diff --git a/cmd/community/validateicons.go b/cmd/community/validateicons.go
@@ -7,6 +7,7 @@ import (
 	"os"
 
 	"github.com/spf13/cobra"
+	"go.starlark.net/starlark"
 	"tidbyt.dev/pixlet/icons"
 	"tidbyt.dev/pixlet/runtime"
 	"tidbyt.dev/pixlet/schema"
@@ -38,8 +39,15 @@ func ValidateIcons(cmd *cobra.Command, args []string) error {
 	runtime.InitHTTP(cache)
 	runtime.InitCache(cache)
 
+	// Remove the print function from the starlark thread.
+	initializers := []runtime.ThreadInitializer{}
+	initializers = append(initializers, func(thread *starlark.Thread) *starlark.Thread {
+		thread.Print = func(thread *starlark.Thread, msg string) {}
+		return thread
+	})
+
 	applet := runtime.Applet{}
-	err = applet.Load(args[0], src, nil)
+	err = applet.LoadWithInitializers(args[0], src, nil, initializers...)
 	if err != nil {
 		return fmt.Errorf("failed to load applet: %w", err)
 	}

diff --git a/cmd/profile.go b/cmd/profile.go
@@ -80,48 +80,64 @@ func profile(cmd *cobra.Command, args []string) error {
 		config[split[0]] = split[1]
 	}
 
+	profile, err := ProfileApp(script, config)
+	if err != nil {
+		return err
+	}
+
+	options := &pprof_driver.Options{
+		Fetch: MakeFetchFunc(profile),
+		UI:    printUI{},
+	}
+	if err = pprof_driver.PProf(options); err != nil {
+		return fmt.Errorf("could not start pprof driver: %w", err)
+	}
+
+	return nil
+}
+
+func ProfileApp(script string, config map[string]string) (*pprof_profile.Profile, error) {
 	src, err := ioutil.ReadFile(script)
 	if err != nil {
-		return fmt.Errorf("failed to read file %s: %w", script, err)
+		return nil, fmt.Errorf("failed to read file %s: %w", script, err)
 	}
 
 	cache := runtime.NewInMemoryCache()
 	runtime.InitHTTP(cache)
 	runtime.InitCache(cache)
 
+	// Remove the print function from the starlark thread.
+	initializers := []runtime.ThreadInitializer{}
+	initializers = append(initializers, func(thread *starlark.Thread) *starlark.Thread {
+		thread.Print = func(thread *starlark.Thread, msg string) {}
+		return thread
+	})
+
 	applet := runtime.Applet{}
-	err = applet.Load(script, src, nil)
+	err = applet.LoadWithInitializers(script, src, nil, initializers...)
 	if err != nil {
-		return fmt.Errorf("failed to load applet: %w", err)
+		return nil, fmt.Errorf("failed to load applet: %w", err)
 	}
 
 	buf := new(bytes.Buffer)
 	if err = starlark.StartProfile(buf); err != nil {
-		return fmt.Errorf("error starting profiler: %w", err)
+		return nil, fmt.Errorf("error starting profiler: %w", err)
 	}
 
-	_, err = applet.Run(config)
+	_, err = applet.Run(config, initializers...)
 	if err != nil {
 		_ = starlark.StopProfile()
-		return fmt.Errorf("error running script: %w", err)
+		return nil, fmt.Errorf("error running script: %w", err)
 	}
 
 	if err = starlark.StopProfile(); err != nil {
-		return fmt.Errorf("error stopping profiler: %w", err)
+		return nil, fmt.Errorf("error stopping profiler: %w", err)
 	}
 
 	profile, err := pprof_profile.ParseData(buf.Bytes())
 	if err != nil {
-		return fmt.Errorf("could not parse pprof profile: %w", err)
-	}
-
-	options := &pprof_driver.Options{
-		Fetch: MakeFetchFunc(profile),
-		UI:    printUI{},
-	}
-	if err = pprof_driver.PProf(options); err != nil {
-		return fmt.Errorf("could not start pprof driver: %w", err)
+		return nil, fmt.Errorf("could not parse pprof profile: %w", err)
 	}
 
-	return nil
+	return profile, nil
 }
diff --git a/cmd/render.go b/cmd/render.go
@@ -100,16 +100,6 @@ func render(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("failed to read file %s: %w", script, err)
 	}
 
-	cache := runtime.NewInMemoryCache()
-	runtime.InitHTTP(cache)
-	runtime.InitCache(cache)
-
-	applet := runtime.Applet{}
-	err = applet.Load(script, src, nil)
-	if err != nil {
-		return fmt.Errorf("failed to load applet: %w", err)
-	}
-
 	// Remove the print function from the starlark thread if the silent flag is
 	// passed.
 	initializers := []runtime.ThreadInitializer{}
@@ -120,6 +110,16 @@ func render(cmd *cobra.Command, args []string) error {
 		})
 	}
 
+	cache := runtime.NewInMemoryCache()
+	runtime.InitHTTP(cache)
+	runtime.InitCache(cache)
+
+	applet := runtime.Applet{}
+	err = applet.LoadWithInitializers(script, src, nil, initializers...)
+	if err != nil {
+		return fmt.Errorf("failed to load applet: %w", err)
+	}
+
 	roots, err := applet.Run(config, initializers...)
 	if err != nil {
 		return fmt.Errorf("error running script: %w", err)

diff --git a/runtime/applet.go b/runtime/applet.go
@@ -92,6 +92,10 @@ func (a *Applet) thread(initializers ...ThreadInitializer) *starlark.Thread {
 // and the actual code should be passed in src. Optionally also pass
 // loader to make additional starlark modules available to the script.
 func (a *Applet) Load(filename string, src []byte, loader ModuleLoader) (err error) {
+	return a.LoadWithInitializers(filename, src, loader)
+}
+
+func (a *Applet) LoadWithInitializers(filename string, src []byte, loader ModuleLoader, initializers ...ThreadInitializer) (err error) {
 	defer func() {
 		if r := recover(); r != nil {
 			err = fmt.Errorf("panic while executing %s: %v", a.Filename, r)
@@ -116,7 +120,7 @@ func (a *Applet) Load(filename string, src []byte, loader ModuleLoader) (err err
 		"struct": starlark.NewBuiltin("struct", starlarkstruct.Make),
 	}
 
-	globals, err := starlark.ExecFile(a.thread(), a.Filename, a.src, a.predeclared)
+	globals, err := starlark.ExecFile(a.thread(initializers...), a.Filename, a.src, a.predeclared)
 	if err != nil {
 		return fmt.Errorf("starlark.ExecFile: %v", err)
 	}