From 17a33b9e23fa9a9fa7c16105b55a5fae698c222e Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Mon, 16 Dec 2019 15:06:27 +0100 Subject: [PATCH] Use string builder to avoid copies on notes rendering We now use the `strings.Builder` to avoid a lot of copies during document rendering. Beside this, we render the markdown in a temporarily buffer before writing it into a file. This will be needed when it comes to the TOC generation which will be an intermediate step. Signed-off-by: Sascha Grunert --- cmd/release-notes/main.go | 11 +++-- pkg/notes/document.go | 83 +++++++++++++++++++++----------------- pkg/notes/document_test.go | 9 +++-- 3 files changed, 57 insertions(+), 46 deletions(-) diff --git a/cmd/release-notes/main.go b/cmd/release-notes/main.go index fcedd3e12c4..3f1bd3a43c4 100644 --- a/cmd/release-notes/main.go +++ b/cmd/release-notes/main.go @@ -294,12 +294,15 @@ func WriteReleaseNotes(releaseNotes notes.ReleaseNotes, history notes.ReleaseNot return errors.Wrapf(err, "creating release note document") } - if err := notes.RenderMarkdown( - output, doc, opts.ReleaseBucket, - opts.ReleaseTars, opts.StartRev, opts.EndRev, - ); err != nil { + markdown, err := notes.RenderMarkdown( + doc, opts.ReleaseBucket, opts.ReleaseTars, opts.StartRev, opts.EndRev, + ) + if err != nil { return errors.Wrapf(err, "rendering release note document to markdown") } + if _, err := output.WriteString(markdown); err != nil { + return errors.Wrapf(err, "writing output file") + } default: return errors.Errorf("%q is an unsupported format", opts.Format) diff --git a/pkg/notes/document.go b/pkg/notes/document.go index 0066dbb474f..03d877ed1a6 100644 --- a/pkg/notes/document.go +++ b/pkg/notes/document.go @@ -111,9 +111,10 @@ func CreateDocument(notes ReleaseNotes, history ReleaseNotesHistory) (*Document, // RenderMarkdown accepts a Document and writes a version of that document to // supplied io.Writer in markdown format. -func RenderMarkdown(w io.Writer, doc *Document, bucket, tars, prevTag, newTag string) error { - if err := createDownloadsTable(w, bucket, tars, prevTag, newTag); err != nil { - return err +func RenderMarkdown(doc *Document, bucket, tars, prevTag, newTag string) (string, error) { + o := &strings.Builder{} + if err := createDownloadsTable(o, bucket, tars, prevTag, newTag); err != nil { + return "", err } // we always want to render the document with SIGs in alphabetical order @@ -123,103 +124,109 @@ func RenderMarkdown(w io.Writer, doc *Document, bucket, tars, prevTag, newTag st } sort.Strings(sortedSIGs) - // this is a helper so that we don't have to check err != nil on every write - - // first, we create a long-lived err that we can re-use - var err error - - // write is a helper that writes a string to the in-scope io.Writer w - write := func(s string) { - // if write has already failed, just return and don't do anything - if err != nil { - return - } - // perform the write - _, err = w.Write([]byte(s)) + nl := func() { + o.WriteRune('\n') + } + nlnl := func() { + nl() + nl() } // writeNote encapsulates the pre-processing that might happen on a note text // before it gets bulleted and written to the io.Writer writeNote := func(s string) { - if !strings.HasPrefix(s, "- ") { - s = "- " + s + const prefix = "- " + if !strings.HasPrefix(s, prefix) { + o.WriteString(prefix) } - write(s + "\n") + o.WriteString(s) + nl() } // the "Action Required" section if len(doc.ActionRequired) > 0 { - write("## Action Required\n\n") + o.WriteString("## Action Required") + nlnl() for _, note := range doc.ActionRequired { writeNote(note) } - write("\n\n") + nlnl() } // the "New Feautres" section if len(doc.NewFeatures) > 0 { - write("## New Features\n\n") + o.WriteString("## New Features") + nlnl() for _, note := range doc.NewFeatures { writeNote(note) } - write("\n\n") + nlnl() } // the "API Changes" section if len(doc.APIChanges) > 0 { - write("### API Changes\n\n") + o.WriteString("### API Changes") + nlnl() for _, note := range doc.APIChanges { writeNote(note) } - write("\n\n") + nlnl() } // the "Duplicate Notes" section if len(doc.Duplicates) > 0 { - write("### Notes from Multiple SIGs\n\n") + o.WriteString("### Notes from Multiple SIGs") + nlnl() for header, notes := range doc.Duplicates { - write(fmt.Sprintf("#### %s\n\n", header)) + o.WriteString("#### ") + o.WriteString(header) + nlnl() for _, note := range notes { writeNote(note) } - write("\n") + nl() } - write("\n") + nl() } // each SIG gets a section (in alphabetical order) if len(sortedSIGs) > 0 { - write("### Notes from Individual SIGs\n\n") + o.WriteString("### Notes from Individual SIGs") + nlnl() for _, sig := range sortedSIGs { - write("#### SIG " + prettySIG(sig) + "\n\n") + o.WriteString("#### SIG ") + o.WriteString(prettySIG(sig)) + nlnl() for _, note := range doc.SIGs[sig] { writeNote(note) } - write("\n") + nl() } - write("\n\n") + nlnl() } // the "Bug Fixes" section if len(doc.BugFixes) > 0 { - write("### Bug Fixes\n\n") + o.WriteString("### Bug Fixes") + nlnl() for _, note := range doc.BugFixes { writeNote(note) } - write("\n\n") + nlnl() } // we call the uncategorized notes "Other Notable Changes". ideally these // notes would at least have a SIG label. if len(doc.Uncategorized) > 0 { - write("### Other Notable Changes\n\n") + o.WriteString("### Other Notable Changes") + nlnl() for _, note := range doc.Uncategorized { writeNote(note) } - write("\n\n") + nlnl() } - return err + return o.String(), nil } // prettySIG takes a sig name as parsed by the `sig-foo` label and returns a diff --git a/pkg/notes/document_test.go b/pkg/notes/document_test.go index 857bccc6390..0c1224e9705 100644 --- a/pkg/notes/document_test.go +++ b/pkg/notes/document_test.go @@ -17,10 +17,10 @@ limitations under the License. package notes import ( - "bytes" "io/ioutil" "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/require" @@ -44,7 +44,6 @@ func TestPrettySIG(t *testing.T) { func TestCreateDownloadsTable(t *testing.T) { // Given - output := &bytes.Buffer{} dir, err := ioutil.TempDir("", "") require.Nil(t, err) defer os.RemoveAll(dir) @@ -80,8 +79,10 @@ func TestCreateDownloadsTable(t *testing.T) { } // When - require.Nil(t, createDownloadsTable(output, "kubernetes-release", dir, - "v1.16.0", "v1.16.1")) + output := &strings.Builder{} + require.Nil(t, createDownloadsTable( + output, "kubernetes-release", dir, "v1.16.0", "v1.16.1", + )) // Then require.Equal(t, output.String(), `# v1.16.1