Memory usage for activation data improved.

[SebastianStehle]

This PR creates a memory optimized version of the Grain lifecycle subject. The ConcurrentDictionary creates a lot of sparse arrays and consumes a lot of memory. In the context of the grain I think that concurrency is not a topic.

With concurrent dictionary (100k Grains)

With simple list (100k Grains)

[jason-bragg]

Good catch.

I agree that the thread safety of a concurrent dictionary is probably not necessary for grains, however having a common lifecycle implementation (or at least the core of the system being shared) reduces code maintenance costs. Duplicating that logic with a different container doubles our code and testing for this system.

For the effects of this to be meaningful the benchmark allocates 100k grains, which are quite a bit more than what tends to be on a single silo.

I think the change in principle is good, but it's not clear the benefit out ways the maintenance cost of duplicate code. Would you mind taking another pass to see if you can keep the majority of the code common.

Thanks

[SebastianStehle]

I was expecting this feedback. For me it seems that there are 3 lifecycle subjects:

• Clients
• Silos
• Grains

For my understanding thread safety is not needed for all three of them, but I am not sure.

[SebastianStehle]

I just changed the base implementation to use a list instead.

[jason-bragg]

Fair point. When initially introduced it was not clear what threads of control would be modifying the lifecycle. Components adding/removing themselves from the lifecycle while it's running was especially a concern (which we've not fully addressed). Using concurrent containers was more of an expedience than a design decision, because the way the systems were exposed it was not clear (and still isn't) how to either enforce thread safe access, or communicate to users in a clear way how they should modify the system which is safe.

If we consider the lifecycle as immutable once started, maybe we should separate the observable lifecycle from the runnable lifecycle and the observable simply be a builder of observer info, which is passed to a runnable lifecycle...?

[ReubenBond]

We could store a simple list of observers & avoid those linq allocations.
Less abstraction is better here, IMHO

[jason-bragg]

I think the linq statements are only an issue due to how long those allocations are around in the current impl. They could easily end up in gen 1 as is, which is not great. IMO, the code is more readable and maintainable using them as compared to hand coding the grouping and sorting, but if we did the organizing in a pass before the main fannout loop they shouldn't be harmful. This, to me, seeams another advantage to separating the observer info building from execution.
Thoughts?

[SebastianStehle]

Before making more improvements it would be great to also consider other feature requests like:

• Feature request: IActivationFilter #4519
• Managing grains without subclassing: Getting the IGrainActivationContext? #5905

I think the grain structure in general is hard to use, because it its not easy to add functionalities and it could also be easier to test grains.

[sergeybykov]

I like this. If we don't have concurrency in the use cases today, no need to pay the perf price. If we might see a concurrent use, should we simply add an optional lock that would be used only if LifecycleSubject is instantiated with a concurrency flag?

Allocations caused by LINQ are a concern. We see some workloads where grains are activated to handle a single request. The impact of such allocations is even worse in those cases.

[SebastianStehle]

The numbers are from a test where 50.000 grains are created, each for one request, but I have not seen the impact of the linq statements in the analysis.

[jason-bragg]

The concurrency issue, imo, is not as much a question of what we do today, because the lifecycle is an extensibility point, but more a question of what do we support. If modification of the lifecycle needs be performed in a single thread, then we need have some means of ensuring that users are aware of this and that patterns we support ensure this.

Grains:
The patterns described in the grain lifecycle docs look to be thread safe. Thoughts?

https://dotnet.github.io/orleans/Documentation/grains/grain_lifecycle.html

Silo
The patterns described in the silo lifecycle docs look to be thread safe. Thoughts?

https://dotnet.github.io/orleans/Documentation/clusters_and_clients/silo_lifecycle.html

The patterns described in the lifecycle core system would require thread safe implementations, but thats mainly describing the abstractions, so we can deal with thread safety when we get there.

https://dotnet.github.io/orleans/Documentation/implementation/orleans_lifecycle.html

From the above, I think it's safe to make this change, as the proscribed patterns are safe with this change. Someone please sanity check me here.

[sergeybykov]

Thank you, @SebastianStehle!