Skip to content
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

Performance updates to SimpleCache, session cache, cache middlewares #2719

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Nov 17, 2022

What was wrong?

Some performance considerations for when to lock the cache.

  • Return the cached value at the same time as the evicted_items rather than reaching back into the cache for it. There's no reason to reach back into the cache for it.
  • Only lock the cache when necessary. I think we were locking the cache a bit too early in almost all cases here. One exception is the async session cache since a session that is cached can be considered stale. That case was left alone as it should be under lock the whole time.

How was it fixed?

  • Lock cache only when necessary.
  • Return the cached value from SimpleCache at the same time as the evicted items.

Todo:

Cute Animal Picture

My beloved former pigmy goat, Bootsy, as a kid:

5

@fselmo fselmo force-pushed the improvements-to-cache-and-simple-cache-middleware branch from 1de52d3 to 81807c7 Compare November 17, 2022 19:50
@fselmo fselmo marked this pull request as ready for review November 17, 2022 20:05
@fselmo fselmo force-pushed the improvements-to-cache-and-simple-cache-middleware branch 2 times, most recently from eb88771 to 5c7d9e3 Compare November 17, 2022 20:10
- Don't lock caches until we have to. If reading from the cache yields a value and a stale value is not an issue, return it. Lock the cache only when writing to it or reading when, say, a stale session is a consideration (async session cache).
@fselmo fselmo force-pushed the improvements-to-cache-and-simple-cache-middleware branch from 5c7d9e3 to 7b1dd30 Compare November 21, 2022 23:51
Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

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

lgtm!

@fselmo fselmo merged commit 92325e2 into ethereum:master Nov 22, 2022
fselmo added a commit to fselmo/web3.py that referenced this pull request Nov 22, 2022
@fselmo fselmo deleted the improvements-to-cache-and-simple-cache-middleware branch April 3, 2024 20:50
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