-
Notifications
You must be signed in to change notification settings - Fork 5
feat: POC of an approach to adding async #78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
77666f8
9a259d3
749ced8
3352006
fa0e661
e17d186
2dc00b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -33,7 +33,12 @@ public void Init(FullDataSet<SerializedItemDescriptor> allData) | |||||||||||||||
{ | ||||||||||||||||
return WaitSafely(() => _coreAsync.GetAsync(kind, key)); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
|
||||||||||||||||
public ValueTask<SerializedItemDescriptor?> GetAsync(DataKind kind, string key,CancellationToken cancellationToken = default) | ||||||||||||||||
{ | ||||||||||||||||
return _coreAsync.GetAsync(kind, key,cancellationToken); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
public KeyedItems<SerializedItemDescriptor> GetAll(DataKind kind) | ||||||||||||||||
{ | ||||||||||||||||
return WaitSafely(() => _coreAsync.GetAllAsync(kind)); | ||||||||||||||||
|
@@ -82,5 +87,14 @@ private T WaitSafely<T>(Func<Task<T>> taskFn) | |||||||||||||||
.GetAwaiter() | ||||||||||||||||
.GetResult(); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
private T WaitSafely<T>(Func<ValueTask<T>> taskFn) | ||||||||||||||||
{ | ||||||||||||||||
return _taskFactory.StartNew(taskFn) | ||||||||||||||||
.GetAwaiter() | ||||||||||||||||
.GetResult() | ||||||||||||||||
.GetAwaiter() | ||||||||||||||||
.GetResult(); | ||||||||||||||||
Comment on lines
+93
to
+97
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we're trying to avoid allocations, maybe something like...
Suggested change
|
||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ | |
using System.Collections.Generic; | ||
using System.Collections.Immutable; | ||
using System.Linq; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using LaunchDarkly.Cache; | ||
using LaunchDarkly.Logging; | ||
using LaunchDarkly.Sdk.Internal; | ||
|
@@ -185,6 +187,22 @@ public void Init(FullDataSet<ItemDescriptor> items) | |
} | ||
} | ||
|
||
public async ValueTask<ItemDescriptor?> GetAsync(DataKind kind, string key, CancellationToken cancellationToken = default) | ||
{ | ||
try | ||
{ | ||
var ret = _itemCache is null ? await GetAndDeserializeItemAsync(kind, key, cancellationToken).ConfigureAwait(false) : | ||
_itemCache.Get(new CacheKey(kind, key)); | ||
Comment on lines
+194
to
+195
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So I was going to say that since the But that made me realize that we don't want an all sync path. The cache loader is async over sync, so we haven't actually avoided any blocking unless we disable the cache. 😭 We'd need to update LaunchDarkly.Cache with |
||
ProcessError(null); | ||
return ret; | ||
} | ||
catch (Exception e) | ||
{ | ||
ProcessError(e); | ||
throw; | ||
} | ||
} | ||
|
||
public KeyedItems<ItemDescriptor> GetAll(DataKind kind) | ||
{ | ||
try | ||
|
@@ -319,7 +337,17 @@ private void Dispose(bool disposing) | |
} | ||
return Deserialize(kind, maybeSerializedItem.Value); | ||
} | ||
|
||
|
||
private async ValueTask<ItemDescriptor?> GetAndDeserializeItemAsync(DataKind kind, string key, CancellationToken cancellationToken = default) | ||
{ | ||
var maybeSerializedItem = await _core.GetAsync(kind, key, cancellationToken).ConfigureAwait(false); | ||
if (!maybeSerializedItem.HasValue) | ||
{ | ||
return null; | ||
} | ||
return Deserialize(kind, maybeSerializedItem.Value); | ||
} | ||
|
||
private ImmutableDictionary<string, ItemDescriptor> GetAllAndDeserialize(DataKind kind) | ||
{ | ||
return _core.GetAll(kind).Items.ToImmutableDictionary( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's not a thing in net 46
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add conditional reference to https://www.nuget.org/packages/System.Threading.Tasks.Extensions for
net462
?