Skip to content

Fix lock contention in FluidValue.Create#432

Merged
sebastienros merged 2 commits intosebastienros:mainfrom
lahma:fix-lock-contention
Jan 2, 2022
Merged

Fix lock contention in FluidValue.Create#432
sebastienros merged 2 commits intosebastienros:mainfrom
lahma:fix-lock-contention

Conversation

@lahma
Copy link
Copy Markdown
Collaborator

@lahma lahma commented Jan 1, 2022

While doing performance checkup in Orchard (load test profiling agency theme) I noticed that there was quite big lock contention problem in Fluid, it originates from #387 where the logic never assigns anything on cache miss thus making the locking and looping happen every time for types like ContentItem (non-dictionary type).

image

Fixed issue by using Interlocked.CompareExchange (no locks needed) and also storing null as result for given type. The dictionary was public, don't know why (Orchard doesn't seem to need it), made it private.

Main

Method Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
CreateFromList 287.9 ns 1.22 ns 1.08 ns 0.0110 - - 184 B

This PR

Method Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
CreateFromList 46.98 ns 0.979 ns 1.048 ns 0.0029 - - 48 B

@lahma lahma changed the title Create benchmarks Fix lock contention in FluidValue.Create Jan 1, 2022
@sebastienros
Copy link
Copy Markdown
Owner

There is a risk with the swap technique that multiple threads can never get to an agreement with all types in the dictionary. Example with two types, A and B.
Thread 1 checks for A, Thread 2 checks for B, none is there, T1 adds A replaces dictionary, T2 adds B, replaces dictionary, A is not there anymore, and this could go on. This is theory, but my previous lock was preventing this. Thread 2 would be blocked until Thread 1 was done.

So ideally the only thing to fix would be the null in the dictionary. In terms of perf no lock would be hit once the cache is warm.

@lahma
Copy link
Copy Markdown
Collaborator Author

lahma commented Jan 1, 2022

https://docs.microsoft.com/en-us/dotnet/api/system.threading.interlocked.compareexchange?view=net-6.0#System_Threading_Interlocked_CompareExchange_System_Object__System_Object_System_Object_ - exchange only happens if field has same reference as in start, if not, someone did successful update and the current race loser will try again. But can also be changed back to double-checked locking.

@sebastienros
Copy link
Copy Markdown
Owner

I understand now, thanks for the explanation. There might be other places I could use that pattern then, I have used the "switch" technique in some places knowing the limitation I was explaining. This technique solves this quite nicely.

@sebastienros sebastienros merged commit 97357f5 into sebastienros:main Jan 2, 2022
@lahma lahma deleted the fix-lock-contention branch January 2, 2022 08:03
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