Threading notes #1551
Mike Stall (MikeStall)
started this conversation in
General
Threading notes
#1551
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
The threading model for Power Fx hosting is basically: writes are single-threaded, reads can be multi-threaded.
While PowerApps architecture is fundamentally single-process-per-user (and single-threaded), any multi-user host (like PVA, Cards, Dataverse) will want to enable a single engine for multiple users and enable parallelism across users (such as a thread per user).
Practically, this means that:
Regarding running evals in parallel - the Fx infrastructure should be threads-safe, such as the parser, lexer, IR translator, C# interpreter, etc. But the Fx objects themselves are not thread safe. So if 2 evals run, both call patch, and mutate the same object, that is not thread-safe.
For threading issues/PRs, we're filling under the Kind_Threading tag.
The host should have full control over the thread of execution - the engine should never introduce new threads. For example, the engine should never call Task.WhenAll (#954).
This is important because hosts may inject objects (such as a connector http channel or Dataverse IOrganizationService) which have limited lifespans and are only valid on the current thread.
TL;DR
readonlyfields and immutable state.[ThreadSafeImmutable]attribute and favoring an immutable pattern.Design
This principal also extends to class design, such as ReadOnlySymbolTable and ReadOnlySymbolValues. For example, the host can create a SymbolTable (which derives from ReadOnlySymbolTable) and call AddVariable to set it up. Once set up, it can pass the ReadOnlySymbolTable to the binder. Hence, the binder can only read and no longer mutate.
This is a similar pattern in the .Net libraries: Dictionary implements IReadOnlyDictionary. Array implements IReadOnlyList.
Avoid mutable shared state.
If state is shared (such as symbol tables), then make it immutable,
Caches
Caches are conceptually readonly, but are still mutations under the hood, and so they still need protection (such as locks).
This has been the cause for bugs like #1208 and #1511.
Also - for any cache, consider what is the cache lifetime strategy? Does it grow monotonically or does it get invalidated?
Concurrent collections
Dictionary is single-threaded. Using a dictionary across multiple threads requires taking a lock around access. .Net offers concurrent collections, like ConcurrentDictionary, which have the lock built in. However ....
Do not preemptively make something multi-threaded.
Understand if code should be single-threaded or multi-threaded. If single-thread, then use Dictionary.
Also consider, if you need to synchronize across multiple collections, you need to still take a lock.
Enforcement
To help enforce the model above, we have some static analysis.
ThreadSafeImmutableAttributemarks a class as conceptually immutable (such as ReadOnlySymbolTable).NotThreadSafemarks a class as mutable (such as SymbolTable).For single-thread region. (like SymbolTable.AddVariable), we have a "Guard" (https://github.com/microsoft/Power-Fx/blob/main/src/libraries/Microsoft.PowerFx.Core/Annotations/GuardSingleThreaded.cs) that will throw when multiple threads try to enter. These are catching hosts bugs. We explicitly do not use a Reader-Writer lock because the goal is not to just make everything multi-threaded. Rather, it is to help the host catch their own bugs and illegal usages.
We also have some stress test that run multiple threaded scenarios to sanity check.
All reactions