Records: prevent object cloning if resulting object is logically the same

[macsux]

Currently, a call to a records with expression always produces a new instance. This however is not optimal if all the new values passed to the with clause are the same as the original record. Consider the following:

public record Node(string MyProperty);
...
var original = new Node("A");
var derived = original with { MyProperty = "A" };
Console.WriteLine(derived.Equals(original)); // true
Console.WriteLine(derived == original); // true
Console.WriteLine(ReferenceEquals(derived, original)); // false

In certain scenarios where happens there's opportunity to reuse existing objects rather than cloning, potentially preventing significant new memory allocations. Since the two objects are immutable, returning original in place of copy should be safe.

This behaviour should probably be on an "opt-in" basis because it would require comparison of the new properties to ones on the existing object before deciding if object needs to be cloned.

A practical example where this might be useful is in code that implements a visitor pattern over immutable data structure (like Roslyn) where 90% of the tree structure could be reused and only small "branch" of nodes changed.

Implementation proposal

I'll update this as discussion below progresses
This change would require the use of upgraded compiler on both side, hence requiring that libraries that choose to use this feature re-target to newer versions of .NET.

Compiler generates a synthetic method like this on record types:

protected virtual bool ShouldClone(string PropertyA, int PropertyB) => true;

If overriden, the compiler would instead of generating the normal hidden parameter-less <Clone>$ method, would instead one that like this:

public virtual A <Clone>$(string PropertyA, int PropertyB)
{
    return ShouldClone(PropertyA, PropertyB) ? new A(this) : this;
}

Consumer side compiler would interpret a with clause like this:

var clone = a with 
{
    PropertyA = "a",
    PropertyB = "b"
}

into something logically similar to this:

var clone = a.<Clone>$("a","b");
clone.PropertyA = "a";
clone.PropertyB = "b";

[HaloFour]

Records are cloned prior to any of the properties being set:

var original = new Node("A");

// the following
var derived = original with { MyProperty = "A" };

// is equivalent to
var $temp = original.Clone();
$temp.MyProperty = "A";
var derived = $temp;

[HaloFour]

Even if .NET were to get something like "immutable classes" (which is a can of worms unto itself) that wouldn't be related to whether "withers" for such types would support this optimization to determine if the instance should be cloned or not. The same problems would exist, in that every possible "wither" would have to incur the cost of such a comparison even if it was not necessary. The value equality component is whether or not such an optimization should be necessary. My opinion is that it's not, especially in the use case of serialization libraries, where the library could be modified to support value equality as well as reference equality through configuration policy.

[macsux]

Well like I mentioned, every "major" project that implements public surface immutability tends to try to optimize for memory allocations internally (I already gave examples of Roslyn, System.Collections.Immutable - and those optimizations are quite elaborate). So in my opinion is that it is something that's tackled when this pattern is applied.

Anyways, going back to original proposal, another idea that could potentially work that wouldn't require messing around with IL hacks is to generate a "hidden" buddy struct to act almost like a builder for data transfer.

Would kinda looks like this from callers perspective:

var builder = instance.<GetBuilder>$();
builder.Prop1 = newValue;
builder.Prop2 = otherValue;
var maybeCloned = builder.Build()

on the record side, how the builder is implemented will be determined by if ShouldClone is overriden:

when not:

public class A 
{
    public A(A copy) {}
    public string PropA {get;set;}

    public Builder <GetBuilder>$(A original) => new <Builder>$(this);
    public struct <Builder>$
    {
        A _clone;
        public <Builder>$(A original)
        {
            _clone = new A(original);
        }
        public string PropA
        {
            get => _clone.PropA;
            set => _clone.PropA = value;
        }
        public A Build() => _clone; // noop really, we already cloned it
    }
}

when overridden:

public class A 
{
    public A(A copy) {}
    public string PropA {get;set;}
    protected override bool ShouldClone(string propA)
    {
        // user logic
        return object.Equals(propA, propA);
    }

    public Builder <GetBuilder>$(A original) => new <Builder>$(this);
    public struct <Builder>$
    {
        A _original;
        
        public <Builder>$(A original)
        {
            _original = original;
        }
        public string PropA
        {
            get; set;
        }
        public A Build() => _original.ShouldClone(PropA) ? new A(_original) : _original;
    }
}

All data passing would happen only on the stack, contract would stay stable and overhead should be negligible, for non overridden scenario as it just acts as passthrough proxy to how current behaviour works.

[HaloFour]

See: https://github.com/dotnet/csharplang/blob/main/meetings/2020/LDM-2020-03-23.md#builder-based-records

[macsux]

See: https://github.com/dotnet/csharplang/blob/main/meetings/2020/LDM-2020-03-23.md#builder-based-records

Very nice. I assume since that was raised in 2020 it stalled and didn't go anywhere?
I cooked up something very similar if you want to take a look while thinking of this problem to handle decision whether to clone or not, though I especially like your approach to tracking set fields in a single member using bit flags

[HaloFour]

Very nice. I assume since that was raised in 2020 it stalled and didn't go anywhere?

No, it didn't go anywhere because it was considered and rejected.

[huoyaoyuan]

Since the two objects are immutable, returning original in place of copy should be safe.

Records are NOT guaranteed immutable:

    public record Node(string MyProperty)
    {
        private int state;

        public int Change() => state++;
    }

And there's no way to detect if a record implementation is indeed immutable.

[macsux]

That's a fair point that they CAN be used implement immutability, but not guaranteed. How about something like this synthesized by the compiler:

protected virtual bool ShouldClone(string PropertyA, int PropertyB) => true;

Default would always clone, but if overriden would allow opt-in behavior to potentially reuse existing instance.

[NN---]

What is the comparison strategy?
Just call field.Equals(newvalue) or something more complicated ?

[macsux]

Well, that would be up to the developer overriding this method. The compiler provided version of it just returns true so existing behaviour stays the same - we just adding a hook to allow it to be altered.