core: Reduce string allocations for addrs Equal

Generating strings and comparing them to implement Equal is a quick and
easy solution. Unfortunately when this code is in the hot path, it
becomes very expensive, so this commit changes some of those instances
to compare the values directly.

Combined with using addr.Equal instead of checking for string equality,
this makes Terraform dramatically faster for some operations, such as
generating large JSON plans.

addrs/module.go

@@ -41,7 +41,15 @@ func (m Module) String() string {
 }
 
 func (m Module) Equal(other Module) bool {
-	return m.String() == other.String()
+	if len(m) != len(other) {
+		return false
+	}
+	for i := range m {
+		if m[i] != other[i] {
+			return false
+		}
+	}
+	return true
 }
 
 func (m Module) targetableSigil() {

addrs/module_instance.go

@@ -268,7 +268,16 @@ func (m ModuleInstance) String() string {
 // Equal returns true if the receiver and the given other value
 // contains the exact same parts.
 func (m ModuleInstance) Equal(o ModuleInstance) bool {
-	return m.String() == o.String()
+	if len(m) != len(o) {
+		return false
+	}
+
+	for i := range m {
+		if m[i] != o[i] {
+			return false
+		}
+	}
+	return true
 }
 
 // Less returns true if the receiver should sort before the given other value

addrs/module_instance_test.go

@@ -0,0 +1,79 @@
+package addrs
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestModuleInstanceEqual_true(t *testing.T) {
+	addrs := []string{
+		"module.foo",
+		"module.foo.module.bar",
+		"module.foo[1].module.bar",
+		`module.foo["a"].module.bar["b"]`,
+		`module.foo["a"].module.bar.module.baz[3]`,
+	}
+	for _, m := range addrs {
+		t.Run(m, func(t *testing.T) {
+			addr, diags := ParseModuleInstanceStr(m)
+			if len(diags) > 0 {
+				t.Fatalf("unexpected diags: %s", diags.Err())
+			}
+			if !addr.Equal(addr) {
+				t.Fatalf("expected %#v to be equal to itself", addr)
+			}
+		})
+	}
+}
+
+func TestModuleInstanceEqual_false(t *testing.T) {
+	testCases := []struct {
+		left  string
+		right string
+	}{
+		{
+			"module.foo",
+			"module.bar",
+		},
+		{
+			"module.foo",
+			"module.foo.module.bar",
+		},
+		{
+			"module.foo[1]",
+			"module.bar[1]",
+		},
+		{
+			`module.foo[1]`,
+			`module.foo["1"]`,
+		},
+		{
+			"module.foo.module.bar",
+			"module.foo[1].module.bar",
+		},
+		{
+			`module.foo.module.bar`,
+			`module.foo["a"].module.bar`,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("%s = %s", tc.left, tc.right), func(t *testing.T) {
+			left, diags := ParseModuleInstanceStr(tc.left)
+			if len(diags) > 0 {
+				t.Fatalf("unexpected diags parsing %s: %s", tc.left, diags.Err())
+			}
+			right, diags := ParseModuleInstanceStr(tc.right)
+			if len(diags) > 0 {
+				t.Fatalf("unexpected diags parsing %s: %s", tc.right, diags.Err())
+			}
+
+			if left.Equal(right) {
+				t.Fatalf("expected %#v not to be equal to %#v", left, right)
+			}
+
+			if right.Equal(left) {
+				t.Fatalf("expected %#v not to be equal to %#v", right, left)
+			}
+		})
+	}
+}

addrs/module_test.go

@@ -0,0 +1,57 @@
+package addrs
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestModuleEqual_true(t *testing.T) {
+	modules := []Module{
+		RootModule,
+		{"a"},
+		{"a", "b"},
+		{"a", "b", "c"},
+	}
+	for _, m := range modules {
+		t.Run(m.String(), func(t *testing.T) {
+			if !m.Equal(m) {
+				t.Fatalf("expected %#v to be equal to itself", m)
+			}
+		})
+	}
+}
+
+func TestModuleEqual_false(t *testing.T) {
+	testCases := []struct {
+		left  Module
+		right Module
+	}{
+		{
+			RootModule,
+			Module{"a"},
+		},
+		{
+			Module{"a"},
+			Module{"b"},
+		},
+		{
+			Module{"a"},
+			Module{"a", "a"},
+		},
+		{
+			Module{"a", "b"},
+			Module{"a", "B"},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("%s = %s", tc.left, tc.right), func(t *testing.T) {
+			if tc.left.Equal(tc.right) {
+				t.Fatalf("expected %#v not to be equal to %#v", tc.left, tc.right)
+			}
+
+			if tc.right.Equal(tc.left) {
+				t.Fatalf("expected %#v not to be equal to %#v", tc.right, tc.left)
+			}
+		})
+	}
+}

addrs/output_value.go

@@ -56,6 +56,10 @@ func (v AbsOutputValue) String() string {
 	return fmt.Sprintf("%s.%s", v.Module.String(), v.OutputValue.String())
 }
 
+func (v AbsOutputValue) Equal(o AbsOutputValue) bool {
+	return v.OutputValue == o.OutputValue && v.Module.Equal(o.Module)
+}
+
 // ModuleCallOutput converts an AbsModuleOutput into a ModuleCallOutput,
 // returning also the module instance that the ModuleCallOutput is relative
 // to.

addrs/output_value_test.go

@@ -0,0 +1,65 @@
+package addrs
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestAbsOutputValueInstanceEqual_true(t *testing.T) {
+	foo, diags := ParseModuleInstanceStr("module.foo")
+	if len(diags) > 0 {
+		t.Fatalf("unexpected diags: %s", diags.Err())
+	}
+	foobar, diags := ParseModuleInstanceStr("module.foo[1].module.bar")
+	if len(diags) > 0 {
+		t.Fatalf("unexpected diags: %s", diags.Err())
+	}
+
+	ovs := []AbsOutputValue{
+		foo.OutputValue("a"),
+		foobar.OutputValue("b"),
+	}
+	for _, r := range ovs {
+		t.Run(r.String(), func(t *testing.T) {
+			if !r.Equal(r) {
+				t.Fatalf("expected %#v to be equal to itself", r)
+			}
+		})
+	}
+}
+
+func TestAbsOutputValueInstanceEqual_false(t *testing.T) {
+	foo, diags := ParseModuleInstanceStr("module.foo")
+	if len(diags) > 0 {
+		t.Fatalf("unexpected diags: %s", diags.Err())
+	}
+	foobar, diags := ParseModuleInstanceStr("module.foo[1].module.bar")
+	if len(diags) > 0 {
+		t.Fatalf("unexpected diags: %s", diags.Err())
+	}
+
+	testCases := []struct {
+		left  AbsOutputValue
+		right AbsOutputValue
+	}{
+		{
+			foo.OutputValue("a"),
+			foo.OutputValue("b"),
+		},
+		{
+			foo.OutputValue("a"),
+			foobar.OutputValue("a"),
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("%s = %s", tc.left, tc.right), func(t *testing.T) {
+			if tc.left.Equal(tc.right) {
+				t.Fatalf("expected %#v not to be equal to %#v", tc.left, tc.right)
+			}
+
+			if tc.right.Equal(tc.left) {
+				t.Fatalf("expected %#v not to be equal to %#v", tc.right, tc.left)
+			}
+		})
+	}
+}

addrs/resource.go

@@ -29,7 +29,7 @@ func (r Resource) String() string {
 }
 
 func (r Resource) Equal(o Resource) bool {
-	return r.String() == o.String()
+	return r.Mode == o.Mode && r.Name == o.Name && r.Type == o.Type
 }
 
 // Instance produces the address for a specific instance of the receiver
@@ -91,7 +91,7 @@ func (r ResourceInstance) String() string {
 }
 
 func (r ResourceInstance) Equal(o ResourceInstance) bool {
-	return r.String() == o.String()
+	return r.Key == o.Key && r.Resource.Equal(o.Resource)
 }
 
 // Absolute returns an AbsResourceInstance from the receiver and the given module
@@ -171,7 +171,7 @@ func (r AbsResource) String() string {
 }
 
 func (r AbsResource) Equal(o AbsResource) bool {
-	return r.String() == o.String()
+	return r.Module.Equal(o.Module) && r.Resource.Equal(o.Resource)
 }
 
 // AbsResourceInstance is an absolute address for a resource instance under a
@@ -236,7 +236,7 @@ func (r AbsResourceInstance) String() string {
 }
 
 func (r AbsResourceInstance) Equal(o AbsResourceInstance) bool {
-	return r.String() == o.String()
+	return r.Module.Equal(o.Module) && r.Resource.Equal(o.Resource)
 }
 
 // Less returns true if the receiver should sort before the given other value
@@ -320,7 +320,7 @@ func (r ConfigResource) String() string {
 }
 
 func (r ConfigResource) Equal(o ConfigResource) bool {
-	return r.String() == o.String()
+	return r.Module.Equal(o.Module) && r.Resource.Equal(o.Resource)
 }
 
 // ResourceMode defines which lifecycle applies to a given resource. Each

addrs/resource_test.go

@@ -0,0 +1,276 @@
+package addrs
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestResourceEqual_true(t *testing.T) {
+	resources := []Resource{
+		{
+			Mode: ManagedResourceMode,
+			Type: "a",
+			Name: "b",
+		},
+		{
+			Mode: DataResourceMode,
+			Type: "a",
+			Name: "b",
+		},
+	}
+	for _, r := range resources {
+		t.Run(r.String(), func(t *testing.T) {
+			if !r.Equal(r) {
+				t.Fatalf("expected %#v to be equal to itself", r)
+			}
+		})
+	}
+}
+
+func TestResourceEqual_false(t *testing.T) {
+	testCases := []struct {
+		left  Resource
+		right Resource
+	}{
+		{
+			Resource{Mode: DataResourceMode, Type: "a", Name: "b"},
+			Resource{Mode: ManagedResourceMode, Type: "a", Name: "b"},
+		},
+		{
+			Resource{Mode: ManagedResourceMode, Type: "a", Name: "b"},
+			Resource{Mode: ManagedResourceMode, Type: "b", Name: "b"},
+		},
+		{
+			Resource{Mode: ManagedResourceMode, Type: "a", Name: "b"},
+			Resource{Mode: ManagedResourceMode, Type: "a", Name: "c"},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("%s = %s", tc.left, tc.right), func(t *testing.T) {
+			if tc.left.Equal(tc.right) {
+				t.Fatalf("expected %#v not to be equal to %#v", tc.left, tc.right)
+			}
+
+			if tc.right.Equal(tc.left) {
+				t.Fatalf("expected %#v not to be equal to %#v", tc.right, tc.left)
+			}
+		})
+	}
+}
+
+func TestResourceInstanceEqual_true(t *testing.T) {
+	resources := []ResourceInstance{
+		{
+			Resource: Resource{
+				Mode: ManagedResourceMode,
+				Type: "a",
+				Name: "b",
+			},
+			Key: IntKey(0),
+		},
+		{
+			Resource: Resource{
+				Mode: DataResourceMode,
+				Type: "a",
+				Name: "b",
+			},
+			Key: StringKey("x"),
+		},
+	}
+	for _, r := range resources {
+		t.Run(r.String(), func(t *testing.T) {
+			if !r.Equal(r) {
+				t.Fatalf("expected %#v to be equal to itself", r)
+			}
+		})
+	}
+}
+
+func TestResourceInstanceEqual_false(t *testing.T) {
+	testCases := []struct {
+		left  ResourceInstance
+		right ResourceInstance
+	}{
+		{
+			ResourceInstance{
+				Resource: Resource{Mode: DataResourceMode, Type: "a", Name: "b"},
+				Key:      IntKey(0),
+			},
+			ResourceInstance{
+				Resource: Resource{Mode: ManagedResourceMode, Type: "a", Name: "b"},
+				Key:      IntKey(0),
+			},
+		},
+		{
+			ResourceInstance{
+				Resource: Resource{Mode: ManagedResourceMode, Type: "a", Name: "b"},
+				Key:      IntKey(0),
+			},
+			ResourceInstance{
+				Resource: Resource{Mode: ManagedResourceMode, Type: "b", Name: "b"},
+				Key:      IntKey(0),
+			},
+		},
+		{
+			ResourceInstance{
+				Resource: Resource{Mode: ManagedResourceMode, Type: "a", Name: "b"},
+				Key:      IntKey(0),
+			},
+			ResourceInstance{
+				Resource: Resource{Mode: ManagedResourceMode, Type: "a", Name: "c"},
+				Key:      IntKey(0),
+			},
+		},
+		{
+			ResourceInstance{
+				Resource: Resource{Mode: DataResourceMode, Type: "a", Name: "b"},
+				Key:      IntKey(0),
+			},
+			ResourceInstance{
+				Resource: Resource{Mode: DataResourceMode, Type: "a", Name: "b"},
+				Key:      StringKey("0"),
+			},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("%s = %s", tc.left, tc.right), func(t *testing.T) {
+			if tc.left.Equal(tc.right) {
+				t.Fatalf("expected %#v not to be equal to %#v", tc.left, tc.right)
+			}
+
+			if tc.right.Equal(tc.left) {
+				t.Fatalf("expected %#v not to be equal to %#v", tc.right, tc.left)
+			}
+		})
+	}
+}
+
+func TestAbsResourceInstanceEqual_true(t *testing.T) {
+	managed := Resource{Mode: ManagedResourceMode, Type: "a", Name: "b"}
+	data := Resource{Mode: DataResourceMode, Type: "a", Name: "b"}
+
+	foo, diags := ParseModuleInstanceStr("module.foo")
+	if len(diags) > 0 {
+		t.Fatalf("unexpected diags: %s", diags.Err())
+	}
+	foobar, diags := ParseModuleInstanceStr("module.foo[1].module.bar")
+	if len(diags) > 0 {
+		t.Fatalf("unexpected diags: %s", diags.Err())
+	}
+
+	instances := []AbsResourceInstance{
+		managed.Instance(IntKey(0)).Absolute(foo),
+		data.Instance(IntKey(0)).Absolute(foo),
+		managed.Instance(StringKey("a")).Absolute(foobar),
+	}
+	for _, r := range instances {
+		t.Run(r.String(), func(t *testing.T) {
+			if !r.Equal(r) {
+				t.Fatalf("expected %#v to be equal to itself", r)
+			}
+		})
+	}
+}
+
+func TestAbsResourceInstanceEqual_false(t *testing.T) {
+	managed := Resource{Mode: ManagedResourceMode, Type: "a", Name: "b"}
+	data := Resource{Mode: DataResourceMode, Type: "a", Name: "b"}
+
+	foo, diags := ParseModuleInstanceStr("module.foo")
+	if len(diags) > 0 {
+		t.Fatalf("unexpected diags: %s", diags.Err())
+	}
+	foobar, diags := ParseModuleInstanceStr("module.foo[1].module.bar")
+	if len(diags) > 0 {
+		t.Fatalf("unexpected diags: %s", diags.Err())
+	}
+
+	testCases := []struct {
+		left  AbsResourceInstance
+		right AbsResourceInstance
+	}{
+		{
+			managed.Instance(IntKey(0)).Absolute(foo),
+			data.Instance(IntKey(0)).Absolute(foo),
+		},
+		{
+			managed.Instance(IntKey(0)).Absolute(foo),
+			managed.Instance(IntKey(0)).Absolute(foobar),
+		},
+		{
+			managed.Instance(IntKey(0)).Absolute(foo),
+			managed.Instance(StringKey("0")).Absolute(foo),
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("%s = %s", tc.left, tc.right), func(t *testing.T) {
+			if tc.left.Equal(tc.right) {
+				t.Fatalf("expected %#v not to be equal to %#v", tc.left, tc.right)
+			}
+
+			if tc.right.Equal(tc.left) {
+				t.Fatalf("expected %#v not to be equal to %#v", tc.right, tc.left)
+			}
+		})
+	}
+}
+
+func TestConfigResourceEqual_true(t *testing.T) {
+	resources := []ConfigResource{
+		{
+			Resource: Resource{Mode: ManagedResourceMode, Type: "a", Name: "b"},
+			Module:   RootModule,
+		},
+		{
+			Resource: Resource{Mode: DataResourceMode, Type: "a", Name: "b"},
+			Module:   RootModule,
+		},
+		{
+			Resource: Resource{Mode: ManagedResourceMode, Type: "a", Name: "b"},
+			Module:   Module{"foo"},
+		},
+		{
+			Resource: Resource{Mode: DataResourceMode, Type: "a", Name: "b"},
+			Module:   Module{"foo"},
+		},
+	}
+	for _, r := range resources {
+		t.Run(r.String(), func(t *testing.T) {
+			if !r.Equal(r) {
+				t.Fatalf("expected %#v to be equal to itself", r)
+			}
+		})
+	}
+}
+
+func TestConfigResourceEqual_false(t *testing.T) {
+	managed := Resource{Mode: ManagedResourceMode, Type: "a", Name: "b"}
+	data := Resource{Mode: DataResourceMode, Type: "a", Name: "b"}
+
+	foo := Module{"foo"}
+	foobar := Module{"foobar"}
+	testCases := []struct {
+		left  ConfigResource
+		right ConfigResource
+	}{
+		{
+			ConfigResource{Resource: managed, Module: foo},
+			ConfigResource{Resource: data, Module: foo},
+		},
+		{
+			ConfigResource{Resource: managed, Module: foo},
+			ConfigResource{Resource: managed, Module: foobar},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("%s = %s", tc.left, tc.right), func(t *testing.T) {
+			if tc.left.Equal(tc.right) {
+				t.Fatalf("expected %#v not to be equal to %#v", tc.left, tc.right)
+			}
+
+			if tc.right.Equal(tc.left) {
+				t.Fatalf("expected %#v not to be equal to %#v", tc.right, tc.left)
+			}
+		})
+	}
+}

plans/changes.go

@@ -51,9 +51,8 @@ func (c *Changes) Empty() bool {
 // resource instance of the given address, if any. Returns nil if no change is
 // planned.
 func (c *Changes) ResourceInstance(addr addrs.AbsResourceInstance) *ResourceInstanceChangeSrc {
-	addrStr := addr.String()
 	for _, rc := range c.Resources {
-		if rc.Addr.String() == addrStr && rc.DeposedKey == states.NotDeposed {
+		if rc.Addr.Equal(addr) && rc.DeposedKey == states.NotDeposed {
 			return rc
 		}
 	}
@@ -81,9 +80,8 @@ func (c *Changes) InstancesForConfigResource(addr addrs.ConfigResource) []*Resou
 // the resource instance of the given address, if any. Returns nil if no change
 // is planned.
 func (c *Changes) ResourceInstanceDeposed(addr addrs.AbsResourceInstance, key states.DeposedKey) *ResourceInstanceChangeSrc {
-	addrStr := addr.String()
 	for _, rc := range c.Resources {
-		if rc.Addr.String() == addrStr && rc.DeposedKey == key {
+		if rc.Addr.Equal(addr) && rc.DeposedKey == key {
 			return rc
 		}
 	}
@@ -94,9 +92,8 @@ func (c *Changes) ResourceInstanceDeposed(addr addrs.AbsResourceInstance, key st
 // OutputValue returns the planned change for the output value with the
 //  given address, if any. Returns nil if no change is planned.
 func (c *Changes) OutputValue(addr addrs.AbsOutputValue) *OutputChangeSrc {
-	addrStr := addr.String()
 	for _, oc := range c.Outputs {
-		if oc.Addr.String() == addrStr {
+		if oc.Addr.Equal(addr) {
 			return oc
 		}
 	}