-
-
Notifications
You must be signed in to change notification settings - Fork 831
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
[image] New image cache #1310
[image] New image cache #1310
Conversation
991484d
to
a82cf03
Compare
b4b70df
to
97257fd
Compare
Rebased on develop and re-tested with tests in PR description |
5566f01
to
c03923e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with multiple images (read with ImageCache + write).
Tested with multiple shared pointer usages.
Tested with multiple threads.
Works fine.
I think the use of a larger type for the ImageCache capacity
and for memory calculations in bytes should be done before the merge.
fa3b2a8
to
5847721
Compare
…r coherent behaviour
…er, imageAlgo::resizeImage for OIIO backend
5847721
to
5acb92a
Compare
Just rebased on develop and force-pushed to make sure there would be no merge conflicts 😄 |
} | ||
|
||
std::string memUsageDesc = "\nMemory usage: " | ||
"\n * capacity: " + std::to_string(_info.capacity) + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need a dedicated function to print in a human readable format, to print with MB, GB, etc depending on the values.
*/ | ||
struct CacheInfo | ||
{ | ||
/// memory usage limits |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
precise unit
return _imagePtrs.at(k).useCount() == 1; | ||
}); | ||
|
||
if (it != _keys.end()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if(it == _keys.end())
{
break;
}
Then the main code don't need to be in a sub-scope, which is better for readability.
} | ||
|
||
// add image to cache if it fits in maxSize | ||
if (memSize + _info.contentSize <= _info.maxSize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, inverse the condition.
The code on the top level scope should be the main one, then you deal with errors in if sub-scope.
if (memSize + _info.contentSize <= _info.maxSize)
{
THROW...
}
load..
return _imagePtrs.at(....):
Description
This PR introduces a new image cache, associated with unit tests and a sample application.
The caching mechanism implemented is a combination of the LRU (least-recently-used) policy and reference counting with shared pointers (as we do not wish to delete images already used in some part of the application).
Features
Tests
The caching mechanism has been tested using 3 image sequences with the following characteristics:
under the following situations (each several time, with varying cache memory limits, number of threads, pixel types and half-sampling levels):