Switched from AsyncDuplicateLock to AsyncKeyedLock

[MarkCiliaVincenti]

Switched from AsyncDuplicateLock to AsyncKeyedLock, which provides better performance and lower memory usage.

[MarkCiliaVincenti]

@nickevansuk

[MarkCiliaVincenti]

@nickevansuk nudge :)

[MarkCiliaVincenti]

@nickevansuk

[nickevansuk]

Thanks for this @MarkCiliaVincenti, very helpful - note that it has previously failed in CI for .NET Framework. Have approved a new run so fingers crossed this time it passes... otherwise it probably just needs a dependency update in the .NET Framework project.

[MarkCiliaVincenti]

Thanks for this @MarkCiliaVincenti, very helpful - note that it has previously failed in CI for .NET Framework. Have approved a new run so fingers crossed this time it passes... otherwise it probably just needs a dependency update in the .NET Framework project.

Does it usually take this long?

[MarkCiliaVincenti]

OK, made some changes, try again now @nickevansuk?

[MarkCiliaVincenti]

@nickevansuk framework ones are failing halfway through. I see it's been a long time since these tests have passed. I'm not convinced the changes I made have broken anything. What are your thoughts?

[MarkCiliaVincenti]

@nickevansuk

[MarkCiliaVincenti]

@nickevansuk any thoughts on this?

[nickevansuk]

So if this requires development effort on our side to fix the .NET Framework stuff it'll need to go into the backlog. I notice that AsyncKeyedLock seems to be evolving quite rapidly - when would you expect it to stabilise? It probably makes most sense for us to look at this once it's settled down.

Also looking at the CI results above it seems that the latest commits produce a syntax error...

[MarkCiliaVincenti]

So if this requires development effort on our side to fix the .NET Framework stuff it'll need to go into the backlog.

Can you please first try https://github.com/openactive/OpenActive.Server.NET/pull/192? If this works, I will merge into this PR.

I notice that AsyncKeyedLock seems to be evolving quite rapidly - when would you expect it to stabilise? It probably makes most sense for us to look at this once it's settled down.

It's stabilised now. The last version came out 3 weeks ago.

Also looking at the CI results above it seems that the latest commits produce a syntax error...

Oops! Fixed now.

[nickevansuk]

Still issues with #192, can add that to the backlog

[MarkCiliaVincenti]

Still issues with #192, can add that to the backlog

I'm not sure I understand what you mean, could you please rephrase?

[nickevansuk]

It looks like there's some issue with .NET Framework that requires some investigation on our side? Is that right? If so have added it to the backlog

[MarkCiliaVincenti]

It looks like there's some issue with .NET Framework that requires some investigation on our side? Is that right? If so have added it to the backlog

There must be. I guess that's a prerequisite for this getting merged?

[nickevansuk]

Afraid so - we can't release something that fails CI

[MarkCiliaVincenti]

@nickevansuk any luck so far?

[nickevansuk]

Nothing yet - hoping to get some time to look in the next couple of weeks. It's a deep issue so needs some proper investigation.

[MarkCiliaVincenti]

Nothing yet - hoping to get some time to look in the next couple of weeks. It's a deep issue so needs some proper investigation.

OK, I updated to 6.2.0 meanwhile although there are no changes that affect you unless you would consider using the new striped locking technique instead.

[MarkCiliaVincenti]

@nickevansuk any luck yet?

[MarkCiliaVincenti]

@nickevansuk do you intend on fixing the persistent issue or shall I close the PR?

[MarkCiliaVincenti]

Can you please try now @nickevansuk ?