Skip to content

Conversation

@richardjrossiii
Copy link
Contributor

Previously, PFKeyValueCache was re-reading the file attributes from disk for every cached entry, every time we attempted to set a new entry in the cache.

This had serious performance impact for use-cases where repeated caching was used, as copying and recreating the file attributes dictionary caused lots of system calls and wasted memory.

This diff makes PFKeyValueCache much more optimistic, as it does not re-read attributes from disk if the containing folder has not changed. However, this does introduce a potential race-condition that is noted in the code.

Bottom line: This should improve query caching performance for 99% of use-cases.

Fixes #141.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: _isDiskCacheDirty

Previously, PFKeyValueCache was re-reading the file attributes from disk for every cached entry, every time we attempted to set a new entry in the cache.

This had serious performance impact for use-cases where repeated caching was used, as copying and recreating the file attributes dictionary caused lots of system calls and wasted memory.

This diff makes PFKeyValueCache much more optimistic, as it does not re-read attributes from disk if the containing folder has not changed. However, this does introduce a potential race-condition that is noted in the code.

Bottom line: This should improve query caching performance for 99% of use-cases.

Fixes #141.
@richardjrossiii richardjrossiii force-pushed the richardross.keyvaluecache.performance branch from 0d93f76 to 9d0a8c1 Compare August 31, 2015 21:00
@nlutsenko
Copy link
Contributor

LGTM. Shipit!

richardjrossiii added a commit that referenced this pull request Sep 1, 2015
…performance

Improved PFKeyValueCache performance and memory usage
@richardjrossiii richardjrossiii merged commit c05b6e8 into master Sep 1, 2015
@richardjrossiii richardjrossiii deleted the richardross.keyvaluecache.performance branch September 1, 2015 05:43
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