[
https://issues.apache.org/jira/browse/ARROW-1795?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16250914#comment-16250914
]
ASF GitHub Bot commented on ARROW-1795:
---------------------------------------
robertnishihara commented on a change in pull request #1317: ARROW-1795:
[Plasma] Fixes to eviction policy.
URL: https://github.com/apache/arrow/pull/1317#discussion_r150742579
##########
File path: cpp/src/plasma/eviction_policy.cc
##########
@@ -67,32 +67,24 @@ int64_t EvictionPolicy::choose_objects_to_evict(int64_t
num_bytes_required,
void EvictionPolicy::object_created(const ObjectID& object_id) {
auto entry = store_info_->objects[object_id].get();
cache_.add(object_id, entry->info.data_size + entry->info.metadata_size);
+ int64_t size = entry->info.data_size + entry->info.metadata_size;
+ memory_used_ += size;
}
bool EvictionPolicy::require_space(int64_t size,
std::vector<ObjectID>* objects_to_evict) {
/* Check if there is enough space to create the object. */
int64_t required_space = memory_used_ + size - store_info_->memory_capacity;
- int64_t num_bytes_evicted;
- if (required_space > 0) {
- /* Try to free up at least as much space as we need right now but ideally
- * up to 20% of the total capacity. */
- int64_t space_to_free = std::max(size, store_info_->memory_capacity / 5);
- ARROW_LOG(DEBUG) << "not enough space to create this object, so evicting
objects";
- /* Choose some objects to evict, and update the return pointers. */
- num_bytes_evicted = choose_objects_to_evict(space_to_free,
objects_to_evict);
- ARROW_LOG(INFO) << "There is not enough space to create this object, so
evicting "
- << objects_to_evict->size() << " objects to free up "
- << num_bytes_evicted << " bytes.";
- } else {
- num_bytes_evicted = 0;
- }
- if (num_bytes_evicted >= required_space) {
- /* We only increment the space used if there is enough space to create the
- * object. */
- memory_used_ += size;
- }
- return num_bytes_evicted >= required_space;
+ /* Try to free up at least as much space as we need right now but ideally
+ * up to 20% of the total capacity. */
+ int64_t space_to_free = std::max(required_space,
store_info_->memory_capacity / 5);
+ ARROW_LOG(DEBUG) << "not enough space to create this object, so evicting
objects";
+ /* Choose some objects to evict, and update the return pointers. */
+ int64_t num_bytes_evicted = choose_objects_to_evict(space_to_free,
objects_to_evict);
+ ARROW_LOG(INFO) << "There is not enough space to create this object, so
evicting "
+ << objects_to_evict->size() << " objects to free up "
+ << num_bytes_evicted << " bytes.";
+ return num_bytes_evicted >= required_space && num_bytes_evicted > 0;
Review comment:
I added the extra condition `num_bytes_evicted > 0` because it is possible
for `require_space` to be called even if `required_space` equals 0.
`require_space` is called whenever `dlmemalign` returns `NULL`, which can
happen even if `required_space` is 0 due to fragmentation and things like that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> [Plasma C++] change evict policy
> --------------------------------
>
> Key: ARROW-1795
> URL: https://issues.apache.org/jira/browse/ARROW-1795
> Project: Apache Arrow
> Issue Type: Improvement
> Components: Plasma (C++)
> Reporter: Lu Qi
> Assignee: Lu Qi
> Priority: Minor
> Labels: pull-request-available
>
> case 1.say, we have total free memory 8 G , we have input 5G data, then comes
> another 6G data,
> if we choose to evict space 6G , it will throw exception saying that
> no object can be free. This is because we didn't count the 3G remaining free
> space .If we count this remaining 3G , we need to ask only 3G,thus
> we can evict the 5G data and we are still alive .
> case 2. another situation is : if we have free memory 10G , we input 1.5G
> data ,then comes another
> 9G data , if we use 10*20% = 2G data to evict ,then we will crash . In this
> situation we need to
> use 9+1.5-10 = 0.5G data to evict
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
