Using ImmutableCollections with Orleans

[Eldar1205]

1. I've read about Orleans [Immutable] attribute and Immutable wrapper to tell the framework not do deep-copy some object.
2. I've read in the technical report that deep copying collections is where the performance hit is at.
3. I've read some of Orleans internal code and there are several places I saw List being copied inside a lock to return a copy of an internal list.

All things above can be addressed better by having support for .Net's new Immutable collections (https://msdn.microsoft.com/en-us/library/system.collections.immutable(v=vs.111).aspx) . It's an external dependency but I believe a good one to have, encouraged to use by .Net as a framework.

1. Does Orleans support application code using Immutable collections in messaging and avoid deep-coping them and have efficient serialization for them?
2. Do you think Orleans should use Immutable collections internally instead of synchronizing lists access and copy lists to achieve immutability?

[jthelin]

The [Immutable] functionality in Orleans was added long time ago before .NET added their own support for immutability. Now it is "legacy" code ;)

Does it make sense to add support for System.Collections.Immutable now, and move to mark the Orleans custom [Immutable] support as [Obsolete] / deprecated for eventual removal in a future release?

[Eldar1205]

The [Immutable] and Immutable<> could still have their usages for non-collection instances. I think about silo local pure functional operations - they shouldn't suffer from any copying cost. On the other hand, a simple to implement algorithm can detect a class has only readonly fields using reflection (https://msdn.microsoft.com/en-us/library/system.reflection.fieldinfo.isinitonly.aspx) or even better detect this at compile time using Roslyn which I'm only familiar with by name

[sergeybykov]

I agree with @Eldar1205 - Immutable is a flexible convention to communicate the intent rather than implementation, which has value beyond the set of concrete immutable classes in System.Collections.Immutable. I think we need both - automatically recognize types from System.Collections.Immutable as immutable and do not deep-copy them, and still support [Immutable]/Immutable<t> for arbitrary types and arguments.

I am a bit cautious about jumping and using those types internally though. We need to benchmark such changes, at least those on the hot paths. We've seen with ConcurrentDictionary that it's not free performance wise. I wonder what's the price of the immutable collections. Allocations?

[gabikliot]

@Eldar1205 , I think recognizing types from System.Collections.Immutable as immutable and not deep-coping them, as @sergeybykov suggested, is a great idea and should also be easy to add. That will solve your first question: "Does Orleans support application code using Immutable collections in messaging and avoid deep-coping them and have efficient serialization for them?"

Would you like to contribute such a PR?

[Eldar1205]

I'll have to dive deeper into Orleans code first and then I'll be glad too. Unfortunately no ETA for that

[gabikliot]

Here are some references:

https://github.com/dotnet/orleans/blob/master/src/Orleans/Serialization/TypeUtilities.cs#L42

https://github.com/dotnet/orleans/blob/master/src/Orleans/Serialization/BuiltInTypes.cs#L74

[jthelin]

FYI, I created a branch with an initial test case to repro the failure mode for ImmutableList.

https://github.com/jthelin/orleans/tree/immutable-types

Unfortunately I have no spare cycles to look deeper into any fixes right now [aka yikes TechFest is next week !]

FWIW, a short-term workaround is to use

list.AsReadOnly();

instead of

list.ToImmutableList();

Both return IList compatible object, so not quite sure why that is not just triggering the general IList serializer in Orleans. https://github.com/dotnet/orleans/blob/master/src/Orleans/Serialization/SerializationManager.cs#L304

BTW, if @Eldar1205 or others want to grab this and run with it, i won't feel offended at all! ;)

[veikkoeeva]

Feels like it's in order to link Microsoft.FSharp.Collections from F# here here and also mention that some types are immutable in F#, such as records (i.e. tuples with labels), Option and so forth. In F#, there are also some semi-official packages such as FSharpx.Collections. I could imagine F# types do cross into silos and there are more users than @ca-ta and @johnazariah.

[gabikliot]

I think I know why it fails. We don't generate serializers for arbitrary system types, by design (see this line - although I have a very hard time to match this if else statement), except for ones we wrote manually, and we did not write one for ImmutableList. I also think it does not trigger the general IList serializer in Orleans by design. I think we are looking for the most accurate type to use. @sergeybykov and @ReubenBond may chime in.

I suspect we just need to manually implement one for ImmutableList, just like we do for ReadOnlyCollection.

[RaringCoder]

I came to poke my nose in about performance. When the immutable collections first appeared, the performance was hit and miss at best. As @sergeybykov has already highlighted, we should be careful about using these defacto. That said, aren't they also OSS? Any performance issues could be tackled to the improvement of both projects.

[Drawaes]

I could fix this quickly, however I am guessing that adding a reference to the System.Collections.Immutable to the Orleans project probably wouldn't be appropriate?

[jason-bragg]

Unless I'm misunderstanding something, System.Collections.Immutable is already a dependency of Microsoft.Orleans.OrleansCodeGenerator. Am I mistaken.

[Drawaes]

I only see a dependency on the OrleansPSUtils in extensions in the solution? I might be wrong but thats what I see in VS.

[jdom]

Just a clarification, it is a (transitive) dependency to our codegen portion, since Roslyn relies on it. We do not use the collections at runtime (perhaps only briefly at startup if you are using runtime codegen, because of Roslyn, but we do not read or write any of them afterwards)

[jdom]

And BTW, several months back I did create a benchmark to validate the performance for each of the collections (mainly for reading, which is the operation we do in our hot path) and it was indeed slower. I'll try to find that benchmark and run it again, and also against the new immutable collections (1.2.0) that were recently released.

[Drawaes]

I don't want to change Olreans to use them. Just to support reference copying instead of a deep copy for grain to grain communications. The semantics would be the same as having a dictionary and wrapping it in the Immutable

[jdom]

Yup, found them and cleaned them up. Pushed here: https://github.com/jdom/ImmutableCollectionsBenchmark

Looks like the read perf for 1.1.37 is equivalent to 1.2.0. Here is a quick summary of the run:

Method	Median	StdDev
DictionaryMutableRead	139.6888 us	1.4509 us
DictionaryImmutableRead	975.4169 us	20.1691 us
ArrayMutableRead	5.7536 us	0.0207 us
ArrayImmutableRead	5.8015 us	0.1064 us
ListMutableRead	8.7713 us	0.0846 us
ListImmutableRead	622.8249 us	1.3053 us

Feel free to look at how the benchmark was performed, as I may have done something wrong

[Drawaes]

Agreed, that Immutables are not good for the hot path in orleans. They have a specific use case (the dictionaries are the same as normal dictionaries) but the structure allows us to make a new mutated version without a complete copy of the underlying dictionary structure. So I have committed support for them from user code, but I am not using them to replace any Orleans structures.

[jdom]

Treating immutable collections as immutable has been implemented by #2067. Thanks @Drawaes. As FYI, that PR did not change Orleans to use immutable collections instead of lists with locks, since quick benchmarks do not justify it, but we'll probably do a perf comparison at some point.