Merge pull request #27998 from hashicorp/alisdair/faster-addr-equals

core: Reduce string allocations for addrs Equal

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
 		}
 	}