From b84fc1eed935cbc943488c7abe57893aa4f6ba40 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Fri, 30 Apr 2021 14:40:46 -0700 Subject: [PATCH] Avoid diffing by lines if inefficient Avoid diffing by lines if it turns out to be significantly less efficient than diffing by bytes. Before this change: ( """ - d5c14bdf6bac81c27afc5429500ed750 - 25483503b557c606dad4f144d27ae10b - 90bdbcdbb6ea7156068e3dcfb7459244 - 978f480a6e3cced51e297fbff9a506b7 + Xd5c14bdf6bac81c27afc5429500ed750 + X25483503b557c606dad4f144d27ae10b + X90bdbcdbb6ea7156068e3dcfb7459244 + X978f480a6e3cced51e297fbff9a506b7 """ ) After this change: strings.Join({ + "X", "d5c14bdf6bac81c27afc5429500ed750\n", + "X", "25483503b557c606dad4f144d27ae10b\n", + "X", "90bdbcdbb6ea7156068e3dcfb7459244\n", + "X", "978f480a6e3cced51e297fbff9a506b7\n", }, "") --- cmp/compare_test.go | 5 +++++ cmp/report_slices.go | 19 +++++++++++++++++-- cmp/testdata/diffs | 12 ++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/cmp/compare_test.go b/cmp/compare_test.go index f7b1f13..c068a3d 100644 --- a/cmp/compare_test.go +++ b/cmp/compare_test.go @@ -1302,6 +1302,11 @@ using the AllowUnexported option.`, "\n"), x: struct{ X interface{} }{[1]string{"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam sit amet pretium ligula, at gravida quam. Integer iaculis, velit at sagittis ultricies, lacus metus scelerisque turpis, ornare feugiat nulla nisl ac erat. Maecenas elementum ultricies libero, sed efficitur lacus molestie non. Nulla ac pretium dolor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Pellentesque mi lorem, consectetur id porttitor id, sollicitudin sit amet enim. Duis eu dolor magna. Nunc ut augue turpis."}}, y: struct{ X interface{} }{[1]string{"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam sit amet pretium ligula, at gravida quam. Integer iaculis, velit at sagittis ultricies, lacus metus scelerisque turpis, ornare feugiat nulla nisl ac erat. Maecenas elementum ultricies libero, sed efficitur lacus molestie non. Nulla ac pretium dolor. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Pellentesque mi lorem, consectetur id porttitor id, sollicitudin sit amet enim. Duis eu dolor magna. Nunc ut augue turpis,"}}, reason: "printing a large standalone string that is different should print enough context to see the difference", + }, { + label: label + "/AllLinesDiffer", + x: "d5c14bdf6bac81c27afc5429500ed750\n25483503b557c606dad4f144d27ae10b\n90bdbcdbb6ea7156068e3dcfb7459244\n978f480a6e3cced51e297fbff9a506b7\n", + y: "Xd5c14bdf6bac81c27afc5429500ed750\nX25483503b557c606dad4f144d27ae10b\nX90bdbcdbb6ea7156068e3dcfb7459244\nX978f480a6e3cced51e297fbff9a506b7\n", + reason: "all lines are different, so diffing based on lines is pointless", }} } diff --git a/cmp/report_slices.go b/cmp/report_slices.go index 168f92f..add2387 100644 --- a/cmp/report_slices.go +++ b/cmp/report_slices.go @@ -98,6 +98,7 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode { // Auto-detect the type of the data. var isLinedText, isText, isBinary bool var sx, sy string + var ssx, ssy []string switch { case t.Kind() == reflect.String: sx, sy = vx.String(), vy.String() @@ -130,6 +131,22 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode { } isText = !isBinary isLinedText = isText && numLines >= 4 && maxLineLen <= 1024 + + // Avoid diffing by lines if it produces a significantly more complex + // edit script than diffing by bytes. + if isLinedText { + ssx = strings.Split(sx, "\n") + ssy = strings.Split(sy, "\n") + esLines := diff.Difference(len(ssx), len(ssy), func(ix, iy int) diff.Result { + return diff.BoolResult(ssx[ix] == ssy[iy]) + }) + esBytes := diff.Difference(len(sx), len(sy), func(ix, iy int) diff.Result { + return diff.BoolResult(sx[ix] == sy[iy]) + }) + efficiencyLines := float64(esLines.Dist()) / float64(len(esLines)) + efficiencyBytes := float64(esBytes.Dist()) / float64(len(esBytes)) + isLinedText = efficiencyLines < 4*efficiencyBytes + } } // Format the string into printable records. @@ -139,8 +156,6 @@ func (opts formatOptions) FormatDiffSlice(v *valueNode) textNode { // If the text appears to be multi-lined text, // then perform differencing across individual lines. case isLinedText: - ssx := strings.Split(sx, "\n") - ssy := strings.Split(sy, "\n") list = opts.formatDiffSlice( reflect.ValueOf(ssx), reflect.ValueOf(ssy), 1, "line", func(v reflect.Value, d diffMode) textRecord { diff --git a/cmp/testdata/diffs b/cmp/testdata/diffs index 05118be..65a645b 100644 --- a/cmp/testdata/diffs +++ b/cmp/testdata/diffs @@ -1046,6 +1046,18 @@ + }, } >>> TestDiff/Reporter/LargeStandaloneString +<<< TestDiff/Reporter/AllLinesDiffer + strings.Join({ ++ "X", + "d5c14bdf6bac81c27afc5429500ed750\n", ++ "X", + "25483503b557c606dad4f144d27ae10b\n", ++ "X", + "90bdbcdbb6ea7156068e3dcfb7459244\n", ++ "X", + "978f480a6e3cced51e297fbff9a506b7\n", + }, "") +>>> TestDiff/Reporter/AllLinesDiffer <<< TestDiff/EmbeddedStruct/ParentStructA/Inequal teststructs.ParentStructA{ privateStruct: teststructs.privateStruct{