Commit 975d51d1 by FritzFlorian

Simplify: use raw pointers instead of optional wrappers on deque.

parent 3687a591
Pipeline #1443 passed with stages
in 3 minutes 33 seconds
......@@ -9,7 +9,6 @@
#include "pls/internal/base/error_handling.h"
#include "pls/internal/base/system_details.h"
#include "pls/internal/data_structures/optional.h"
#include "pls/internal/data_structures/stamped_integer.h"
#include "pls/internal/scheduling/lock_free/task.h"
......@@ -38,8 +37,8 @@ class external_trading_deque {
public:
external_trading_deque(unsigned thread_id, size_t num_entries) : thread_id_(thread_id), entries_(num_entries) {}
static optional<task *> peek_traded_object(task *target_task);
static optional<task *> get_trade_object(task *target_task);
static task *peek_traded_object(task *target_task);
static task *get_trade_object(task *target_task);
/**
* Pushes a task on the bottom of the deque.
......@@ -54,12 +53,12 @@ class external_trading_deque {
*
* @return optional<task*> holding the popped task if successful.
*/
optional<task *> pop_bot();
task *pop_bot();
struct peek_result {
peek_result(optional<task *> top_task, stamped_integer top_pointer) : top_task_{std::move(top_task)},
top_pointer_{top_pointer} {};
optional<task *> top_task_;
peek_result(task *top_task, stamped_integer top_pointer) : top_task_{std::move(top_task)},
top_pointer_{top_pointer} {};
task *top_task_;
stamped_integer top_pointer_;
};
......@@ -83,7 +82,7 @@ class external_trading_deque {
*
* @return optional<task*> holding the popped task if successful.
*/
optional<task *> pop_top(task *offered_task, peek_result peek_result);
task *pop_top(task *offered_task, peek_result peek_result);
private:
void reset_bot_and_top();
......
......@@ -3,26 +3,26 @@
namespace pls::internal::scheduling::lock_free {
optional<task *> external_trading_deque::peek_traded_object(task *target_task) {
task * external_trading_deque::peek_traded_object(task *target_task) {
traded_cas_field current_cas = target_task->external_trading_deque_cas_.load();
if (current_cas.is_filled_with_object()) {
return optional<task *>{current_cas.get_trade_object()};
return current_cas.get_trade_object();
} else {
return optional<task *>{};
return nullptr;
}
}
optional<task *> external_trading_deque::get_trade_object(task *target_task) {
task * external_trading_deque::get_trade_object(task *target_task) {
traded_cas_field current_cas = target_task->external_trading_deque_cas_.load();
if (current_cas.is_filled_with_object()) {
task *result = current_cas.get_trade_object();
traded_cas_field empty_cas;
if (target_task->external_trading_deque_cas_.compare_exchange_strong(current_cas, empty_cas)) {
return optional<task *>{result};
return result;
}
}
return optional<task *>{};
return nullptr;
}
void external_trading_deque::push_bot(task *published_task) {
......@@ -58,10 +58,10 @@ void external_trading_deque::decrease_bot() {
bot_.store(bot_internal_.value, std::memory_order_relaxed);
}
optional<task *> external_trading_deque::pop_bot() {
task * external_trading_deque::pop_bot() {
if (bot_internal_.value == 0) {
reset_bot_and_top();
return optional<task *>{};
return nullptr;
}
decrease_bot();
......@@ -77,10 +77,10 @@ optional<task *> external_trading_deque::pop_bot() {
if (popped_task->external_trading_deque_cas_.compare_exchange_strong(expected_sync_cas_field,
empty_cas_field,
std::memory_order_acq_rel)) {
return optional<task *>{popped_task};
return popped_task;
} else {
reset_bot_and_top();
return optional<task *>{};
return nullptr;
}
}
......@@ -89,17 +89,17 @@ external_trading_deque::peek_result external_trading_deque::peek_top() {
auto local_bot = bot_.load();
if (local_top.value < local_bot) {
return peek_result{optional<task *>{entries_[local_top.value].traded_task_}, local_top};
return peek_result{entries_[local_top.value].traded_task_, local_top};
} else {
return peek_result{optional<task *>{}, local_top};
return peek_result{nullptr, local_top};
}
}
optional<task *> external_trading_deque::pop_top(task *offered_task, peek_result peek_result) {
task * external_trading_deque::pop_top(task *offered_task, peek_result peek_result) {
stamped_integer expected_top = peek_result.top_pointer_;
auto local_bot = bot_.load();
if (expected_top.value >= local_bot) {
return data_structures::optional<task *>{};
return nullptr;
}
auto &target_entry = entries_[expected_top.value];
......@@ -119,7 +119,7 @@ optional<task *> external_trading_deque::pop_top(task *offered_task, peek_result
// We got it, for sure move the top pointer forward.
top_.compare_exchange_strong(expected_top, {expected_top.stamp + 1, expected_top.value + 1});
// Return the stolen task
return data_structures::optional<task *>{result};
return result;
} else {
// We did not get it...help forwarding the top pointer anyway.
if (expected_top.stamp == forwarding_stamp) {
......@@ -130,7 +130,7 @@ optional<task *> external_trading_deque::pop_top(task *offered_task, peek_result
// This means only updating the tag, as this location can still hold data we need.
top_.compare_exchange_strong(expected_top, {forwarding_stamp, expected_top.value});
}
return data_structures::optional<task *>{};
return nullptr;
}
}
......
......@@ -36,12 +36,7 @@ void task_manager::push_local_task(base_task *pushed_task) {
}
base_task *task_manager::pop_local_task() {
auto result = deque_.pop_bot();
if (result) {
return *result;
} else {
return nullptr;
}
return deque_.pop_bot();
}
std::tuple<base_task *, base_task *> task_manager::steal_task(thread_state &stealing_state) {
......@@ -50,27 +45,27 @@ std::tuple<base_task *, base_task *> task_manager::steal_task(thread_state &stea
auto peek = deque_.peek_top();
if (peek.top_task_) {
task *stolen_task = static_cast<task *>(*peek.top_task_);
task *stolen_task = peek.top_task_;
// get a suitable task to trade in
// TODO: opt. add debug marker to traded in tasks that we do not accidentally use them.
task *traded_task = static_cast<task *>(&scheduler::task_chain_at(stolen_task->depth_, stealing_state));
base_task *chain_after_stolen_task = traded_task->next_;
// perform the actual pop operation
auto pop_result_task = deque_.pop_top(traded_task, peek);
task* pop_result_task = deque_.pop_top(traded_task, peek);
if (pop_result_task) {
PLS_ASSERT(stolen_task->thread_id_ != traded_task->thread_id_,
"It is impossible to steal an task we already own!");
PLS_ASSERT(*pop_result_task == stolen_task,
PLS_ASSERT(pop_result_task == stolen_task,
"We must only steal the task that we peeked at!");
// update the resource stack associated with the stolen task
stolen_task->push_task_chain(traded_task);
auto optional_exchanged_task = external_trading_deque::get_trade_object(stolen_task);
task* optional_exchanged_task = external_trading_deque::get_trade_object(stolen_task);
if (optional_exchanged_task) {
// All good, we pushed the task over to the stack, nothing more to do
PLS_ASSERT(*optional_exchanged_task == traded_task,
PLS_ASSERT(optional_exchanged_task == traded_task,
"We are currently executing this, no one else can put another task in this field!");
} else {
// The last other active thread took it as its spare resource...
......@@ -93,9 +88,9 @@ base_task *task_manager::pop_clean_task_chain(base_task *base_task) {
task *clean_chain = popped_task->pop_task_chain();
if (clean_chain == nullptr) {
// double-check if we are really last one or we only have unlucky timing
auto optional_cas_task = external_trading_deque::get_trade_object(popped_task);
task* optional_cas_task = external_trading_deque::get_trade_object(popped_task);
if (optional_cas_task) {
clean_chain = *optional_cas_task;
clean_chain = optional_cas_task;
} else {
clean_chain = popped_task->pop_task_chain();
}
......
......@@ -81,30 +81,30 @@ TEST_CASE("external trading deque", "[internal/scheduling/lock_free/external_tra
// Local push/pop
deque_1.push_bot(&tasks[0]);
REQUIRE(*deque_1.pop_bot() == &tasks[0]);
REQUIRE(deque_1.pop_bot() == &tasks[0]);
REQUIRE(!deque_1.pop_bot());
// Local push, external pop
deque_1.push_bot(&tasks[0]);
auto peek = deque_1.peek_top();
REQUIRE(*deque_1.pop_top(&tasks[1], peek) == &tasks[0]);
REQUIRE(*external_trading_deque::get_trade_object(&tasks[0]) == &tasks[1]);
REQUIRE(deque_1.pop_top(&tasks[1], peek) == &tasks[0]);
REQUIRE(external_trading_deque::get_trade_object(&tasks[0]) == &tasks[1]);
REQUIRE(!deque_1.pop_top(&tasks[1], peek));
REQUIRE(!deque_1.pop_bot());
// Keeps push/pop order
deque_1.push_bot(&tasks[0]);
deque_1.push_bot(&tasks[1]);
REQUIRE(*deque_1.pop_bot() == &tasks[1]);
REQUIRE(*deque_1.pop_bot() == &tasks[0]);
REQUIRE(deque_1.pop_bot() == &tasks[1]);
REQUIRE(deque_1.pop_bot() == &tasks[0]);
REQUIRE(!deque_1.pop_bot());
deque_1.push_bot(&tasks[0]);
deque_1.push_bot(&tasks[1]);
auto peek1 = deque_1.peek_top();
REQUIRE(*deque_1.pop_top(&tasks[2], peek1) == &tasks[0]);
REQUIRE(deque_1.pop_top(&tasks[2], peek1) == &tasks[0]);
auto peek2 = deque_1.peek_top();
REQUIRE(*deque_1.pop_top(&tasks[3], peek2) == &tasks[1]);
REQUIRE(deque_1.pop_top(&tasks[3], peek2) == &tasks[1]);
}
SECTION("Interwined execution #1") {
......@@ -112,7 +112,7 @@ TEST_CASE("external trading deque", "[internal/scheduling/lock_free/external_tra
deque_1.push_bot(&tasks[0]);
auto peek1 = deque_1.peek_top();
auto peek2 = deque_1.peek_top();
REQUIRE(*deque_1.pop_top(&tasks[1], peek1) == &tasks[0]);
REQUIRE(deque_1.pop_top(&tasks[1], peek1) == &tasks[0]);
REQUIRE(!deque_1.pop_top(&tasks[2], peek2));
}
......@@ -120,7 +120,7 @@ TEST_CASE("external trading deque", "[internal/scheduling/lock_free/external_tra
// Top and bottom access
deque_1.push_bot(&tasks[0]);
auto peek1 = deque_1.peek_top();
REQUIRE(*deque_1.pop_bot() == &tasks[0]);
REQUIRE(deque_1.pop_bot() == &tasks[0]);
REQUIRE(!deque_1.pop_top(&tasks[2], peek1));
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment