Skip to content

Commit

Permalink
Avoid diffing by lines if inefficient
Browse files Browse the repository at this point in the history
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",
    }, "")
  • Loading branch information
dsnet committed Apr 30, 2021
1 parent 8fa37b4 commit b84fc1e
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 2 deletions.
5 changes: 5 additions & 0 deletions cmp/compare_test.go
Expand Up @@ -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",
}}
}

Expand Down
19 changes: 17 additions & 2 deletions cmp/report_slices.go
Expand Up @@ -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()
Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand Down
12 changes: 12 additions & 0 deletions cmp/testdata/diffs
Expand Up @@ -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{
Expand Down

0 comments on commit b84fc1e

Please sign in to comment.