Skip to content

Like-for-like replacement of KeyedLock with AsyncKeyedLock library. - #21

Merged
elsand merged 11 commits into
data-altinn-no:masterfrom
MarkCiliaVincenti:AsyncKeyedLock
Dec 17, 2022
Merged

elsand merged 11 commits into
data-altinn-no:masterfrom
MarkCiliaVincenti:AsyncKeyedLock

Conversation

@MarkCiliaVincenti

Copy link
Copy Markdown
Contributor

Made a like-for-like replacement of the KeyedLock with a more performant AsyncKeyedLock library. You may want to review the pool size or remove it altogether, but I left it with the value of 10 to be just like KeyedLock was.

@MarkCiliaVincenti

Copy link
Copy Markdown
Contributor Author

@elsand

@MarkCiliaVincenti

Copy link
Copy Markdown
Contributor Author

@elsand

@elsand elsand left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, the library looks good and we would like to integrate this. However, we cannot delete the current KeyedLock implementation from Dan.Common, as this namespace is deployed as a nuget and removing it will require a major version bump. Please re-add KeyedLock.cs, and I will merge this.

@MarkCiliaVincenti

Copy link
Copy Markdown
Contributor Author

I've re-implemented KeyedLock using AsyncKeyedLocker internally. I'm not sure if you want to add an [Obsolete] to KeyedLock.

@MarkCiliaVincenti

Copy link
Copy Markdown
Contributor Author

Oops sorry about that @elsand, realized I hadn't actually hit the save button on the changes I did, you can see them now. Once more, I'm not sure if you want to add an [Obsolete] to KeyedLock.

@elsand

elsand commented Dec 13, 2022

Copy link
Copy Markdown
Member

I've re-implemented KeyedLock using AsyncKeyedLocker internally

Very nice, thanks :)

I'm not sure if you want to add an [Obsolete] to KeyedLock.

Yes, that's a good idea.

@MarkCiliaVincenti

Copy link
Copy Markdown
Contributor Author

I've re-implemented KeyedLock using AsyncKeyedLocker internally

Very nice, thanks :)

I'm not sure if you want to add an [Obsolete] to KeyedLock.

Yes, that's a good idea.

OK, done. Over to you to merge.

@MarkCiliaVincenti

Copy link
Copy Markdown
Contributor Author

@elsand

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants