Better handling of Future handles #81
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s [BROKEN] - Future handles are no longer stored in safe handles, to prevent the GC from destroying the futures behind our back - FdbFutureContext is the base class of all "contexts" that can have futures (ie: clusters, transactions, ...) - There is a static dictionary that stores all active contexts (first routing step) - Each context has also its own dictionary that stores all active futures in this context (second rounting step) - future callback parameter is the concatenation of the context ID and the future ID and is used for the two-step routing - Multi-futures (FdbFutureArray<T>) use refcounting and only fire once the last handle has fired
…llbacks - maybe remove this when we are done?
…hread, and defer the handling to the thread pool - When callback are invoked inline, we have to return ASAP to the caller (who holds a lock) so we move to the TP - If we have already been moved to the TP, then we can call OnReady inline (already on the TP!) else we defer to the TP at that time. - Use the 32 lower bits as the key in the future dictionary, because IntPtr is not IEquatable<IntPtr> and uses a boxing key comparer - Added a "dead" flag to future context, and implemented refcounting down to 0 to remove the context from the global MapToException - Changed the way transaction are cancelled, by waiting for FDB_C to call us back for each pending future (need to map `transaction_cancelled` into a TaskCancelledException for this to work) - Started working on a Watch refactoring
…ost of copying the execution context for nothing - Used internally to trigger cancellation, and we do not care about the ExecutionContext to do that.
…d transaction - Cancellation will be handled by tr.Cancel() internally
…t is done, and more locking
- Using a profiler, and looking at the x64 assembly generated by the JIT, it seems that always calling memcmp / memmove is fast enough - Added #define MEASURE to count and measure the invocation of memory copy and compares to get a feel of the most used sizes by typical applications - Count 0, 1 and 2, which are most impacted by calling memcmp or memcpy, are less frequent. Most frequent in a reference app where size 5 (corresponds to a 4 bytes integer + the type prefix in tuple encoding) or 17 bytes (GUIDs in tuples). - Reduced the number of method calls needed to invoke memcmp and memmove - Made EnsureSliceIsValid and EnsureBufferIsValid inlined by default, to remove one more method call
- byte* ptr =&buffer[offset] : does a bound check which can fail if the array is empty. - byte* ptr; ptr + offset : does not bound check
…t possible for byte copy and compare: - The x64 assembly generated to get the addess of &left[offset] is smaller and faster, than getting the addess of left and adding the offset manually - &left[offset] also does a nullcheck and boundcheck at runtime "for free"
…when possible - Encoding.UTF8 returns a new object everytime it is called, so cache one and reuse it - Use new string(sbyte*, ..., [Encoding]) instead of [Encoding].GetString(byte[], .....) whenever possible
# Conflicts: # FoundationDB.Client/Core/IFdbTransactionHandler.cs # FoundationDB.Client/Fdb.cs # FoundationDB.Client/FdbTransaction.Snapshot.cs # FoundationDB.Client/FdbTransaction.cs # FoundationDB.Client/FdbWatch.cs # FoundationDB.Client/FoundationDB.Client.csproj # FoundationDB.Client/Native/FdbFuture.cs # FoundationDB.Client/Native/FdbFutureArray.cs # FoundationDB.Client/Native/FdbFutureSingle.cs # FoundationDB.Client/Native/FdbNative.cs # FoundationDB.Client/Native/FdbNativeCluster.cs # FoundationDB.Client/Native/FdbNativeTransaction.cs # FoundationDB.Client/Subspaces/Fdb.Directory.cs # FoundationDB.Client/Tuples/Encoding/TupleParser.cs # FoundationDB.Client/Utils/Slice.cs # FoundationDB.Client/Utils/SliceComparer.cs # FoundationDB.Client/Utils/SliceHelpers.cs # FoundationDB.Tests/Layers/TupleFacts.cs # FoundationDB.Tests/TransactionFacts.cs # FoundationDB.Tests/Utils/SliceFacts.cs
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is tracking PR for the work of rewriting the way FDBFuture* handles are wrapped to .NET's
Task<T>, as detailed in #48. Original work was done in #54 but was rebased on master hereThis PR changes the following:
FdbFutureHandlehas been removed, and Futures handles are now back to IntPtr. This helps avoid problems when the GC wants to destroy handles at unwanted times.IFdbFuture, so that they can be put in the same dictionary or list.