From c40dc7ec34e19f67f4426deb0a3f1915d68e0d17 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Tue, 6 Jun 2023 10:43:52 -0700 Subject: [PATCH 1/6] img optimizations --- lib/imgbundler/imgbundler.go | 25 ++++++++--- lib/imgbundler/imgbundler_test.go | 73 +++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/lib/imgbundler/imgbundler.go b/lib/imgbundler/imgbundler.go index 5c6de7038..50e3b126b 100644 --- a/lib/imgbundler/imgbundler.go +++ b/lib/imgbundler/imgbundler.go @@ -23,6 +23,8 @@ import ( "oss.terrastruct.com/util-go/xmain" ) +var imgCache sync.Map + const maxImageSize int64 = 1 << 25 // 33_554_432 var imageRegex = regexp.MustCompile(`ab +`, url1, url2) + + ms := &xmain.State{ + Name: "test", + + Stdin: os.Stdin, + Stdout: os.Stdout, + Stderr: os.Stderr, + + Env: xos.NewEnv(os.Environ()), + } + ms.Log = cmdlog.NewTB(ms.Env, t) + + count := 0 + + httpClient.Transport = roundTripFunc(func(req *http.Request) *http.Response { + count++ + respRecorder := httptest.NewRecorder() + respRecorder.WriteString(`\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n`) + respRecorder.WriteHeader(200) + return respRecorder.Result() + }) + + out, err := BundleRemote(ctx, ms, []byte(sampleSVG)) + if err != nil { + t.Fatal(err) + } + tassert.Equal(t, 1, count) + if strings.Contains(string(out), url1) { + t.Fatal("links still exist") + } + tassert.Equal(t, 2, strings.Count(string(out), "image/svg+xml")) +} From b37b5571e14a1e0f19c768d7a1c99b97407e9d6e Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Tue, 6 Jun 2023 10:56:44 -0700 Subject: [PATCH 2/6] logs --- lib/imgbundler/imgbundler.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/imgbundler/imgbundler.go b/lib/imgbundler/imgbundler.go index 50e3b126b..d7df77851 100644 --- a/lib/imgbundler/imgbundler.go +++ b/lib/imgbundler/imgbundler.go @@ -111,7 +111,7 @@ func runWorkers(ctx context.Context, ms *xmain.State, svg []byte, imgs [][][]byt <-sema }() - bundledImage, err := worker(ctx, &imgCache, img[1], isRemote) + bundledImage, err := worker(ctx, ms, img[1], isRemote) if err != nil { ms.Log.Error.Printf("failed to bundle %s: %v", img[1], err) errhrefsMu.Lock() @@ -150,16 +150,18 @@ func runWorkers(ctx context.Context, ms *xmain.State, svg []byte, imgs [][][]byt } } -func worker(ctx context.Context, cache *sync.Map, href []byte, isRemote bool) ([]byte, error) { - if hit, ok := cache.Load(string(href)); ok { +func worker(ctx context.Context, ms *xmain.State, href []byte, isRemote bool) ([]byte, error) { + if hit, ok := imgCache.Load(string(href)); ok { return hit.([]byte), nil } var buf []byte var mimeType string var err error if isRemote { + ms.Log.Debug.Printf("fetching %s remotely", string(href)) buf, mimeType, err = httpGet(ctx, html.UnescapeString(string(href))) } else { + ms.Log.Debug.Printf("reading %s from disk", string(href)) buf, err = os.ReadFile(html.UnescapeString(string(href))) } if err != nil { @@ -173,7 +175,7 @@ func worker(ctx context.Context, cache *sync.Map, href []byte, isRemote bool) ([ b64 := base64.StdEncoding.EncodeToString(buf) out := []byte(fmt.Sprintf(` Date: Tue, 6 Jun 2023 11:09:19 -0700 Subject: [PATCH 3/6] flag --- ci/release/template/man/d2.1 | 3 +++ d2cli/main.go | 7 +++++++ lib/imgbundler/imgbundler.go | 10 +++++++--- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/ci/release/template/man/d2.1 b/ci/release/template/man/d2.1 index 81bc4e895..ff8263105 100644 --- a/ci/release/template/man/d2.1 +++ b/ci/release/template/man/d2.1 @@ -109,6 +109,9 @@ An appendix for tooltips and links is added to PNG exports since they are not in .Ns . .It Fl d , -debug Print debug logs. +.It Fl -img-cache Ar true +In watch mode, images used in icons are cached for subsequent compilations. This should be disabled if images might change +.Ns . .It Fl h , -help Print usage information and exit. .It Fl v , -version diff --git a/d2cli/main.go b/d2cli/main.go index 0db873bc6..9b5f1cc8a 100644 --- a/d2cli/main.go +++ b/d2cli/main.go @@ -67,6 +67,10 @@ func Run(ctx context.Context, ms *xmain.State) (err error) { if err != nil { return err } + imgCacheFlag, err := ms.Opts.Bool("IMG_CACHE", "img-cache", "", true, "in watch mode, images used in icons are cached for subsequent compilations. This should be disabled if images might change.") + if err != nil { + return err + } layoutFlag := ms.Opts.String("D2_LAYOUT", "layout", "l", "dagre", `the layout engine used`) themeFlag, err := ms.Opts.Int64("D2_THEME", "theme", "t", 0, "the diagram theme ID") if err != nil { @@ -150,6 +154,9 @@ func Run(ctx context.Context, ms *xmain.State) (err error) { if *debugFlag { ms.Env.Setenv("DEBUG", "1") } + if *imgCacheFlag { + ms.Env.Setenv("IMG_CACHE", "1") + } if *browserFlag != "" { ms.Env.Setenv("BROWSER", *browserFlag) } diff --git a/lib/imgbundler/imgbundler.go b/lib/imgbundler/imgbundler.go index d7df77851..526f3bf8d 100644 --- a/lib/imgbundler/imgbundler.go +++ b/lib/imgbundler/imgbundler.go @@ -151,8 +151,10 @@ func runWorkers(ctx context.Context, ms *xmain.State, svg []byte, imgs [][][]byt } func worker(ctx context.Context, ms *xmain.State, href []byte, isRemote bool) ([]byte, error) { - if hit, ok := imgCache.Load(string(href)); ok { - return hit.([]byte), nil + if ms.Env.Getenv("IMG_CACHE") == "1" { + if hit, ok := imgCache.Load(string(href)); ok { + return hit.([]byte), nil + } } var buf []byte var mimeType string @@ -175,7 +177,9 @@ func worker(ctx context.Context, ms *xmain.State, href []byte, isRemote bool) ([ b64 := base64.StdEncoding.EncodeToString(buf) out := []byte(fmt.Sprintf(` Date: Tue, 6 Jun 2023 11:17:02 -0700 Subject: [PATCH 4/6] add test for cache --- lib/imgbundler/imgbundler_test.go | 80 +++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/lib/imgbundler/imgbundler_test.go b/lib/imgbundler/imgbundler_test.go index 4625a4f85..49acdb7d3 100644 --- a/lib/imgbundler/imgbundler_test.go +++ b/lib/imgbundler/imgbundler_test.go @@ -294,3 +294,83 @@ width="328" height="587" viewBox="-100 -131 328 587">ab +`, url1, url2) + + ms := &xmain.State{ + Name: "test", + + Stdin: os.Stdin, + Stdout: os.Stdout, + Stderr: os.Stderr, + + Env: xos.NewEnv(os.Environ()), + } + ms.Log = cmdlog.NewTB(ms.Env, t) + + count := 0 + + httpClient.Transport = roundTripFunc(func(req *http.Request) *http.Response { + count++ + respRecorder := httptest.NewRecorder() + respRecorder.WriteString(`\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n`) + respRecorder.WriteHeader(200) + return respRecorder.Result() + }) + + // Using a cache, imgs are not refetched on multiple runs + ms.Env.Setenv("IMG_CACHE", "1") + _, err := BundleRemote(ctx, ms, []byte(sampleSVG)) + if err != nil { + t.Fatal(err) + } + _, err = BundleRemote(ctx, ms, []byte(sampleSVG)) + if err != nil { + t.Fatal(err) + } + tassert.Equal(t, 1, count) + + // With cache disabled, it refetches + ms.Env.Setenv("IMG_CACHE", "0") + count = 0 + _, err = BundleRemote(ctx, ms, []byte(sampleSVG)) + if err != nil { + t.Fatal(err) + } + _, err = BundleRemote(ctx, ms, []byte(sampleSVG)) + if err != nil { + t.Fatal(err) + } + tassert.Equal(t, 2, count) +} From 8123f8525a63db4675d5630c86355ba68fd0d439 Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Tue, 6 Jun 2023 11:19:26 -0700 Subject: [PATCH 5/6] changelog --- ci/release/changelogs/next.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/release/changelogs/next.md b/ci/release/changelogs/next.md index ef58f531f..de58d5b7a 100644 --- a/ci/release/changelogs/next.md +++ b/ci/release/changelogs/next.md @@ -11,6 +11,8 @@ - `sql_table` now alternatively takes an array of constraints instead of being limited to a single one. Thanks @satoqz ! [#1245](https://github.com/terrastruct/d2/pull/1245) - Constraints in `sql_table` render even if they have no matching abbreviation [#1372](https://github.com/terrastruct/d2/pull/1372) - Constraints in `sql_table` sheds their excessive letter-spacing and is padded from the end consistently [#1372](https://github.com/terrastruct/d2/pull/1372) +- Duplicate image URLs in icons are only fetched once [#1373](https://github.com/terrastruct/d2/pull/1373) +- In watch mode, images are cached by default across compiles. Can be disabled with flag `--img-cache=0`. [#1373](https://github.com/terrastruct/d2/pull/1373) #### Bugfixes ⛑️ From 79e192f9c1d86dd3ace7f564c6b29b87593e875b Mon Sep 17 00:00:00 2001 From: Alexander Wang Date: Tue, 6 Jun 2023 11:33:00 -0700 Subject: [PATCH 6/6] fmt --- lib/imgbundler/imgbundler_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/imgbundler/imgbundler_test.go b/lib/imgbundler/imgbundler_test.go index 49acdb7d3..c2eddb21a 100644 --- a/lib/imgbundler/imgbundler_test.go +++ b/lib/imgbundler/imgbundler_test.go @@ -14,6 +14,7 @@ import ( "testing" tassert "github.com/stretchr/testify/assert" + "oss.terrastruct.com/util-go/cmdlog" "oss.terrastruct.com/util-go/xos"