Script: Simplify implementation of ScriptList

Use btrees instead of set/map
Replace bucket mechanism for reverse mapping
Avoid redundant operations
This commit is contained in:
Jonathan G Rennison
2023-08-03 22:05:26 +01:00
parent b68491f2fe
commit bdbc631e6c
3 changed files with 234 additions and 138 deletions

View File

@@ -50,7 +50,7 @@ public:
*/
bool IsEnd()
{
return this->list->buckets.empty() || this->has_no_more_items;
return this->list->items.empty() || this->has_no_more_items;
}
/**
@@ -58,6 +58,16 @@ public:
*/
virtual void Remove(SQInteger item) = 0;
/**
* Callback from the list after an item gets removed.
*/
virtual void PostErase(SQInteger item, ScriptList::ScriptListMap::iterator post_erase, ScriptList::ScriptListValueSet::iterator value_post_erase) = 0;
/**
* Callback from the list if an item's value is changed.
*/
virtual void ValueChange(SQInteger item) = 0;
/**
* Attach the sorter to a new list. This assumes the content of the old list has been moved to
* the new list, too, so that we don't have to invalidate any iterators. Note that std::swap
@@ -75,9 +85,7 @@ public:
*/
class ScriptListSorterValueAscending : public ScriptListSorter {
private:
ScriptList::ScriptListBucket::iterator bucket_iter; ///< The iterator over the list to find the buckets.
ScriptList::ScriptItemList *bucket_list; ///< The current bucket list we're iterator over.
ScriptList::ScriptItemList::iterator bucket_list_iter; ///< The iterator over the bucket list.
ScriptList::ScriptListValueSet::iterator value_iter; ///< The iterator over the value list.
public:
/**
@@ -92,13 +100,11 @@ public:
SQInteger Begin()
{
if (this->list->buckets.empty()) return 0;
if (this->list->values.empty()) return 0;
this->has_no_more_items = false;
this->bucket_iter = this->list->buckets.begin();
this->bucket_list = &(*this->bucket_iter).second;
this->bucket_list_iter = this->bucket_list->begin();
this->item_next = *this->bucket_list_iter;
this->value_iter = this->list->values.begin();
this->item_next = (*this->value_iter).second;
SQInteger item_current = this->item_next;
FindNext();
@@ -107,7 +113,6 @@ public:
void End()
{
this->bucket_list = nullptr;
this->has_no_more_items = true;
this->item_next = 0;
}
@@ -117,22 +122,12 @@ public:
*/
void FindNext()
{
if (this->bucket_list == nullptr) {
if (this->value_iter == this->list->values.end()) {
this->has_no_more_items = true;
return;
}
this->bucket_list_iter++;
if (this->bucket_list_iter == this->bucket_list->end()) {
this->bucket_iter++;
if (this->bucket_iter == this->list->buckets.end()) {
this->bucket_list = nullptr;
return;
}
this->bucket_list = &(*this->bucket_iter).second;
this->bucket_list_iter = this->bucket_list->begin();
}
this->item_next = *this->bucket_list_iter;
this->value_iter++;
if (this->value_iter != this->list->values.end()) item_next = (*this->value_iter).second;
}
SQInteger Next()
@@ -154,6 +149,24 @@ public:
return;
}
}
void PostErase(SQInteger item, ScriptList::ScriptListMap::iterator post_erase, ScriptList::ScriptListValueSet::iterator value_post_erase)
{
if (this->IsEnd()) return;
/*
* This is to the optimise the case where the current item is removed, and the resulting iterator points to the expected next item.
* NB: A positive generation means that the iterator was not the end iterator, and therefore that item_next has a valid value.
*/
if (value_post_erase != this->list->values.end() && this->value_iter.generation() > 0 && value_post_erase->second == this->item_next) {
this->value_iter = value_post_erase;
}
}
void ValueChange(SQInteger item)
{
this->ScriptListSorterValueAscending::Remove(item);
}
};
/**
@@ -164,9 +177,7 @@ private:
/* Note: We cannot use reverse_iterator.
* The iterators must only be invalidated when the element they are pointing to is removed.
* This only holds for forward iterators. */
ScriptList::ScriptListBucket::iterator bucket_iter; ///< The iterator over the list to find the buckets.
ScriptList::ScriptItemList *bucket_list; ///< The current bucket list we're iterator over.
ScriptList::ScriptItemList::iterator bucket_list_iter; ///< The iterator over the bucket list.
ScriptList::ScriptListValueSet::iterator value_iter; ///< The iterator over the value list.
public:
/**
@@ -181,18 +192,12 @@ public:
SQInteger Begin()
{
if (this->list->buckets.empty()) return 0;
if (this->list->values.empty()) return 0;
this->has_no_more_items = false;
/* Go to the end of the bucket-list */
this->bucket_iter = this->list->buckets.end();
--this->bucket_iter;
this->bucket_list = &(*this->bucket_iter).second;
/* Go to the end of the items in the bucket */
this->bucket_list_iter = this->bucket_list->end();
--this->bucket_list_iter;
this->item_next = *this->bucket_list_iter;
this->value_iter = this->list->values.end();
--this->value_iter;
this->item_next = this->value_iter->second;
SQInteger item_current = this->item_next;
FindNext();
@@ -201,7 +206,6 @@ public:
void End()
{
this->bucket_list = nullptr;
this->has_no_more_items = true;
this->item_next = 0;
}
@@ -211,25 +215,17 @@ public:
*/
void FindNext()
{
if (this->bucket_list == nullptr) {
if (this->value_iter == this->list->values.end()) {
this->has_no_more_items = true;
return;
}
if (this->bucket_list_iter == this->bucket_list->begin()) {
if (this->bucket_iter == this->list->buckets.begin()) {
this->bucket_list = nullptr;
return;
}
this->bucket_iter--;
this->bucket_list = &(*this->bucket_iter).second;
/* Go to the end of the items in the bucket */
this->bucket_list_iter = this->bucket_list->end();
--this->bucket_list_iter;
if (this->value_iter == this->list->values.begin()) {
/* Use 'end' as marker for 'beyond begin' */
this->value_iter = this->list->values.end();
} else {
this->bucket_list_iter--;
this->value_iter--;
}
this->item_next = *this->bucket_list_iter;
if (this->value_iter != this->list->values.end()) item_next = this->value_iter->second;
}
SQInteger Next()
@@ -251,6 +247,16 @@ public:
return;
}
}
void PostErase(SQInteger item, ScriptList::ScriptListMap::iterator post_erase, ScriptList::ScriptListValueSet::iterator value_post_erase)
{
/* not implemented */
}
void ValueChange(SQInteger item)
{
this->ScriptListSorterValueDescending::Remove(item);
}
};
/**
@@ -321,6 +327,24 @@ public:
return;
}
}
void PostErase(SQInteger item, ScriptList::ScriptListMap::iterator post_erase, ScriptList::ScriptListValueSet::iterator value_post_erase)
{
if (this->IsEnd()) return;
/*
* This is to the optimise the case where the current item is removed, and the resulting iterator points to the expected next item.
* NB: A positive generation means that the iterator was not the end iterator, and therefore that item_next has a valid value.
*/
if (post_erase != this->list->items.end() && this->item_iter.generation() > 0 && post_erase->first == this->item_next) {
this->item_iter = post_erase;
}
}
void ValueChange(SQInteger item)
{
/* do nothing */
}
};
/**
@@ -400,6 +424,16 @@ public:
return;
}
}
void PostErase(SQInteger item, ScriptList::ScriptListMap::iterator post_erase, ScriptList::ScriptListValueSet::iterator value_post_erase)
{
/* not implemented */
}
void ValueChange(SQInteger item)
{
/* do nothing */
}
};
@@ -429,18 +463,76 @@ void ScriptList::Clear()
this->modifications++;
this->items.clear();
this->buckets.clear();
this->values.clear();
this->sorter->End();
}
void ScriptList::AddOrSetItem(SQInteger item, SQInteger value)
{
auto res = this->items.insert(std::make_pair(item, value));
if (!res.second) {
/* Key was already present, insertion did not take place */
this->SetIterValue(res.first, value);
return;
}
this->modifications++;
this->values.insert(std::make_pair(value, item));
}
void ScriptList::AddToItemValue(SQInteger item, SQInteger value)
{
auto res = this->items.insert(std::make_pair(item, value));
if (!res.second) {
/* Key was already present, insertion did not take place */
this->SetIterValue(res.first, res.first->second + value);
return;
}
this->modifications++;
this->values.insert(std::make_pair(value, item));
}
void ScriptList::AddItem(SQInteger item, SQInteger value)
{
this->modifications++;
if (this->HasItem(item)) return;
auto res = this->items.insert(std::make_pair(item, value));
if (!res.second) {
/* Key was already present, insertion did not take place */
return;
}
this->items[item] = value;
this->buckets[value].insert(item);
this->values.insert(std::make_pair(value, item));
}
ScriptList::ScriptListMap::iterator ScriptList::RemoveIter(ScriptList::ScriptListMap::iterator item_iter)
{
SQInteger item = item_iter->first;
SQInteger value = item_iter->second;
this->sorter->Remove(item);
ScriptListMap::iterator new_item_iter = this->items.erase(item_iter);
ScriptListValueSet::iterator new_reverse_iter = this->values.erase(this->values.find(std::make_pair(value, item)));
this->sorter->PostErase(item, new_item_iter, new_reverse_iter);
return new_item_iter;
}
ScriptList::ScriptListValueSet::iterator ScriptList::RemoveValueIter(ScriptList::ScriptListValueSet::iterator value_iter)
{
SQInteger item = value_iter->second;
this->sorter->Remove(item);
ScriptListMap::iterator new_item_iter = this->items.erase(this->items.find(item));
ScriptListValueSet::iterator new_value_iter = this->values.erase(value_iter);
this->sorter->PostErase(item, new_item_iter, new_value_iter);
return new_value_iter;
}
void ScriptList::RemoveItem(SQInteger item)
@@ -450,14 +542,7 @@ void ScriptList::RemoveItem(SQInteger item)
ScriptListMap::iterator item_iter = this->items.find(item);
if (item_iter == this->items.end()) return;
SQInteger value = item_iter->second;
this->sorter->Remove(item);
ScriptListBucket::iterator bucket_iter = this->buckets.find(value);
assert(bucket_iter != this->buckets.end());
bucket_iter->second.erase(item);
if (bucket_iter->second.empty()) this->buckets.erase(bucket_iter);
this->items.erase(item_iter);
this->RemoveIter(item_iter);
}
SQInteger ScriptList::Begin()
@@ -496,10 +581,25 @@ SQInteger ScriptList::Count()
SQInteger ScriptList::GetValue(SQInteger item)
{
ScriptListMap::const_iterator item_iter = this->items.find(item);
ScriptListMap::iterator item_iter = this->items.find(item);
return item_iter == this->items.end() ? 0 : item_iter->second;
}
void ScriptList::SetIterValue(ScriptListMap::iterator item_iter, SQInteger value)
{
SQInteger value_old = item_iter->second;
if (value_old == value) return;
SQInteger item = item_iter->first;
this->sorter->ValueChange(item);
this->values.erase(this->values.find(std::make_pair(value_old, item)));
item_iter->second = value;
this->values.insert(std::make_pair(value, item));
}
bool ScriptList::SetValue(SQInteger item, SQInteger value)
{
this->modifications++;
@@ -507,16 +607,7 @@ bool ScriptList::SetValue(SQInteger item, SQInteger value)
ScriptListMap::iterator item_iter = this->items.find(item);
if (item_iter == this->items.end()) return false;
SQInteger value_old = item_iter->second;
if (value_old == value) return true;
this->sorter->Remove(item);
ScriptListBucket::iterator bucket_iter = this->buckets.find(value_old);
assert(bucket_iter != this->buckets.end());
bucket_iter->second.erase(item);
if (bucket_iter->second.empty()) this->buckets.erase(bucket_iter);
item_iter->second = value;
this->buckets[value].insert(item);
this->SetIterValue(item_iter, value);
return true;
}
@@ -560,7 +651,7 @@ void ScriptList::AddList(ScriptList *list)
if (this->IsEmpty()) {
/* If this is empty, we can just take the items of the other list as is. */
this->items = list->items;
this->buckets = list->buckets;
this->values = list->values;
this->modifications++;
} else {
ScriptListMap *list_items = &list->items;
@@ -576,7 +667,7 @@ void ScriptList::SwapList(ScriptList *list)
if (list == this) return;
this->items.swap(list->items);
this->buckets.swap(list->buckets);
this->values.swap(list->values);
Swap(this->sorter, list->sorter);
Swap(this->sorter_type, list->sorter_type);
Swap(this->sort_ascending, list->sort_ascending);
@@ -640,24 +731,16 @@ void ScriptList::RemoveTop(SQInteger count)
switch (this->sorter_type) {
default: NOT_REACHED();
case SORT_BY_VALUE:
for (ScriptListBucket::iterator iter = this->buckets.begin(); iter != this->buckets.end(); iter = this->buckets.begin()) {
ScriptItemList *items = &(*iter).second;
size_t size = items->size();
for (ScriptItemList::iterator iter = items->begin(); iter != items->end(); iter = items->begin()) {
for (ScriptListValueSet::iterator iter = this->values.begin(); iter != this->values.end(); iter = this->values.begin()) {
if (--count < 0) return;
this->RemoveItem(*iter);
/* When the last item is removed from the bucket, the bucket itself is removed.
* This means that the iterators can be invalid after a call to RemoveItem.
*/
if (--size == 0) break;
}
this->RemoveValueIter(iter);
}
break;
case SORT_BY_ITEM:
for (ScriptListMap::iterator iter = this->items.begin(); iter != this->items.end(); iter = this->items.begin()) {
if (--count < 0) return;
this->RemoveItem((*iter).first);
this->RemoveIter(iter);
}
break;
}
@@ -677,24 +760,18 @@ void ScriptList::RemoveBottom(SQInteger count)
switch (this->sorter_type) {
default: NOT_REACHED();
case SORT_BY_VALUE:
for (ScriptListBucket::reverse_iterator iter = this->buckets.rbegin(); iter != this->buckets.rend(); iter = this->buckets.rbegin()) {
ScriptItemList *items = &(*iter).second;
size_t size = items->size();
for (ScriptItemList::reverse_iterator iter = items->rbegin(); iter != items->rend(); iter = items->rbegin()) {
for (ScriptListValueSet::iterator iter = this->values.end(); iter != this->values.begin(); iter = this->values.end()) {
if (--count < 0) return;
this->RemoveItem(*iter);
/* When the last item is removed from the bucket, the bucket itself is removed.
* This means that the iterators can be invalid after a call to RemoveItem.
*/
if (--size == 0) break;
}
--iter;
this->RemoveValueIter(iter);
}
break;
case SORT_BY_ITEM:
for (ScriptListMap::reverse_iterator iter = this->items.rbegin(); iter != this->items.rend(); iter = this->items.rbegin()) {
for (ScriptListMap::iterator iter = this->items.end(); iter != this->items.begin(); iter = this->items.end()) {
if (--count < 0) return;
this->RemoveItem((*iter).first);
--iter;
this->RemoveIter(iter);
}
break;
}
@@ -707,9 +784,9 @@ void ScriptList::RemoveList(ScriptList *list)
if (list == this) {
Clear();
} else {
ScriptListMap *list_items = &list->items;
for (ScriptListMap::iterator iter = list_items->begin(); iter != list_items->end(); iter++) {
this->RemoveItem((*iter).first);
ScriptListMap &list_items = list->items;
for (ScriptListMap::iterator iter = list_items.begin(); iter != list_items.end(); iter++) {
this->RemoveItem(iter->first);
}
}
}
@@ -718,9 +795,12 @@ void ScriptList::KeepAboveValue(SQInteger value)
{
this->modifications++;
for (ScriptListMap::iterator next_iter, iter = this->items.begin(); iter != this->items.end(); iter = next_iter) {
next_iter = iter; next_iter++;
if ((*iter).second <= value) this->RemoveItem((*iter).first);
for (ScriptListMap::iterator iter = this->items.begin(); iter != this->items.end();) {
if (iter->second <= value) {
iter = this->RemoveIter(iter);
} else {
++iter;
}
}
}
@@ -728,9 +808,12 @@ void ScriptList::KeepBelowValue(SQInteger value)
{
this->modifications++;
for (ScriptListMap::iterator next_iter, iter = this->items.begin(); iter != this->items.end(); iter = next_iter) {
next_iter = iter; next_iter++;
if ((*iter).second >= value) this->RemoveItem((*iter).first);
for (ScriptListMap::iterator iter = this->items.begin(); iter != this->items.end();) {
if (iter->second >= value) {
iter = this->RemoveIter(iter);
} else {
++iter;
}
}
}
@@ -738,9 +821,12 @@ void ScriptList::KeepBetweenValue(SQInteger start, SQInteger end)
{
this->modifications++;
for (ScriptListMap::iterator next_iter, iter = this->items.begin(); iter != this->items.end(); iter = next_iter) {
next_iter = iter; next_iter++;
if ((*iter).second <= start || (*iter).second >= end) this->RemoveItem((*iter).first);
for (ScriptListMap::iterator iter = this->items.begin(); iter != this->items.end();) {
if (iter->second <= start || iter->second >= end) {
iter = this->RemoveIter(iter);
} else {
++iter;
}
}
}
@@ -748,9 +834,12 @@ void ScriptList::KeepValue(SQInteger value)
{
this->modifications++;
for (ScriptListMap::iterator next_iter, iter = this->items.begin(); iter != this->items.end(); iter = next_iter) {
next_iter = iter; next_iter++;
if ((*iter).second != value) this->RemoveItem((*iter).first);
for (ScriptListMap::iterator iter = this->items.begin(); iter != this->items.end();) {
if (iter->second != value) {
iter = this->RemoveIter(iter);
} else {
++iter;
}
}
}
@@ -809,12 +898,7 @@ SQInteger ScriptList::_set(HSQUIRRELVM vm)
}
sq_getinteger(vm, 3, &val);
if (!this->HasItem(idx)) {
this->AddItem(idx, val);
return 0;
}
this->SetValue(idx, val);
this->AddOrSetItem(idx, val);
return 0;
}
@@ -869,7 +953,7 @@ SQInteger ScriptList::Valuate(HSQUIRRELVM vm)
/* Push the function to call */
sq_push(vm, 2);
for (ScriptListMap::iterator iter = this->items.begin(); iter != this->items.end(); iter++) {
for (ScriptListMap::iterator iter = this->items.begin(); iter != this->items.end(); ++iter) {
/* Check for changing of items. */
int previous_modification_count = this->modifications;
@@ -930,7 +1014,7 @@ SQInteger ScriptList::Valuate(HSQUIRRELVM vm)
return sq_throwerror(vm, "modifying valuated list outside of valuator function");
}
this->SetValue((*iter).first, value);
this->SetIterValue(iter, value);
/* Pop the return value. */
sq_poptop(vm);

View File

@@ -12,8 +12,8 @@
#define SCRIPT_LIST_HPP
#include "script_object.hpp"
#include <map>
#include <set>
#include "../../3rdparty/cpp-btree/safe_btree_set.h"
#include "../../3rdparty/cpp-btree/safe_btree_map.h"
class ScriptListSorter;
@@ -34,6 +34,13 @@ public:
/** Sort descending */
static const bool SORT_DESCENDING = false;
/**
* The safe btree variants ars used because these automatically manage refreshing iterators
* which have been invalidated by adding/removing items.
*/
typedef btree::safe_btree_map<SQInteger, SQInteger> ScriptListMap; ///< Key to value map
typedef btree::safe_btree_set<std::pair<SQInteger, SQInteger>> ScriptListValueSet; ///< [Value, Key] set
private:
ScriptListSorter *sorter; ///< Sorting algorithm
SorterType sorter_type; ///< Sorting type
@@ -41,13 +48,13 @@ private:
bool initialized; ///< Whether an iteration has been started
int modifications; ///< Number of modification that has been done. To prevent changing data while valuating.
public:
typedef std::set<SQInteger> ScriptItemList; ///< The list of items inside the bucket
typedef std::map<SQInteger, ScriptItemList> ScriptListBucket; ///< The bucket list per value
typedef std::map<SQInteger, SQInteger> ScriptListMap; ///< List per item
void SetIterValue(ScriptListMap::iterator item_iter, SQInteger value);
ScriptListMap::iterator RemoveIter(ScriptListMap::iterator item_iter);
ScriptListValueSet::iterator RemoveValueIter(ScriptListValueSet::iterator value_iter);
public:
ScriptListMap items; ///< The items in the list
ScriptListBucket buckets; ///< The items in the list, sorted by value
ScriptListValueSet values; ///< The items in the list, sorted by value
ScriptList();
~ScriptList();
@@ -63,6 +70,16 @@ public:
void AddItem(SQInteger item, SQInteger value = 0);
#endif /* DOXYGEN_API */
/**
* @api -all
*/
void AddOrSetItem(SQInteger item, SQInteger value);
/**
* @api -all
*/
void AddToItemValue(SQInteger item, SQInteger value_to_add);
/**
* Remove a single item from the list.
* @param item the item to remove. If not existing, it is ignored.

View File

@@ -133,12 +133,7 @@ CargoCollector::~CargoCollector()
void CargoCollector::SetValue()
{
if (this->amount > 0) {
if (this->list->HasItem(this->last_key)) {
this->list->SetValue(this->last_key,
this->list->GetValue(this->last_key) + this->amount);
} else {
this->list->AddItem(this->last_key, this->amount);
}
this->list->AddToItemValue(this->last_key, this->amount);
}
}