Skip to content

Lock free Read/Write Identical Request Implementation#124

Merged
JimBobSquarePants merged 2 commits intoSixLabors:js/faster-cachefrom
ThisNetWorks:dm/lock-free-cache
Sep 17, 2020
Merged

Lock free Read/Write Identical Request Implementation#124
JimBobSquarePants merged 2 commits intoSixLabors:js/faster-cachefrom
ThisNetWorks:dm/lock-free-cache

Conversation

@deanmarcussen
Copy link
Copy Markdown
Collaborator

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Proposes replacing the AsyncKeyLock with an implementation based around ConcurrentDictionary.

Note: I have not removed the AsyncKeyLock code or related test code.

Test coverage from the Integration Tests.

Copy link
Copy Markdown
Member

@JimBobSquarePants JimBobSquarePants left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@JimBobSquarePants JimBobSquarePants merged commit 434b149 into SixLabors:js/faster-cache Sep 17, 2020
@cristipufu
Copy link
Copy Markdown

@deanmarcussen @JimBobSquarePants can you guys please provide more details about this change? How is it better (pros/cons) vs the previous implementation with the AsyncLock?

@JimBobSquarePants
Copy link
Copy Markdown
Member

It’s about 30x faster as I recall from the load test benchmarks

@sebastienros
Copy link
Copy Markdown
Contributor

To be precise, under stress it went from 2K RPS to 60K RPS, which was the static file middleware raw perf.

@deanmarcussen
Copy link
Copy Markdown
Collaborator Author

deanmarcussen commented Dec 13, 2020

Worth having a read of the full pr from @JimBobSquarePants #121 where the bottlenecks in the existing lock where identified and we tested the lock free approach

@cristipufu
Copy link
Copy Markdown

Thanks a lot guys!

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.

4 participants