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

Replace SemispaceCache with LRU Cache #1163

Merged
merged 8 commits into from
Jan 16, 2025
Merged

Replace SemispaceCache with LRU Cache #1163

merged 8 commits into from
Jan 16, 2025

Conversation

mpilquist
Copy link
Member

@mpilquist mpilquist commented Jan 15, 2025

This PR removes SemispaceCache and adds a least recently used cache (creatively named Cache). This improves cache behavior, especially with access patterns where cached entries were promoted from gen 1 to gen 0 and resultantly lots of cached statements would be evicted.

The LRU implementation is based on maps and access stamps -- not particularly storage efficient but should be good enough for this use case. I thought about implementing this with a splay tree instead but opted for the hash map design.

The configured max cache space is accurately enforced now, whereas SemispaceCache would store up to twice the configured max. Hence, the session defaults were all doubled to get the same net maximum.

Eviction tracking is NOT built in to Cache like it was in SemispaceCache. Instead, putting an element in to a cache returns the element that was evicted as a result, if any. Eviction tracking moved up in to StatementCache instead.

cc @matthughes

Copy link
Contributor

@matthughes matthughes left a comment

Choose a reason for hiding this comment

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

LGTM. I built and ran this PR against my integration tests and regular runtime and did not see any issues. Confirmed that it does not create more pg_prepared_statements than cache limit.

@mpilquist mpilquist merged commit 1884e71 into main Jan 16, 2025
21 checks passed
@mpilquist mpilquist deleted the topic/lru-cache branch January 16, 2025 19: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