Skip to content
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

Fixes, move spinlock to cc, minor changes #42

Open
wants to merge 61 commits into
base: develop
Choose a base branch
from
Open

Conversation

jkunstwald
Copy link
Contributor

  • Move td spinlock to cc, use that version (cc::spin_lock)
  • Move new functions out of experimental namespace
  • Allocator support in MPMCQueue
  • Various fixes
    • Default init handle::counter
    • Fix null_counter_value
  • Deprecate and clean up num_physical_cores
    • This is not trivial to do correctly on Linux, current solution is Intel only and doesn't account for disabled (but available) hyperthreading

@jkunstwald jkunstwald changed the title Kw/develop Fixes, move spinlock to cc, minor changes Oct 1, 2021
::DWORD length = 0;
std::byte buffer_stack[4096];
cc::array<std::byte> buffer_heap;
DWORD buffer_length = CC_COUNTOF(buffer_stack);
Copy link
Contributor

Choose a reason for hiding this comment

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

hm, could we think about renaming CC_COUNTOF? it's a bit too short and ambiguous for my tastes. especially for something that is actually not used that often

@@ -16,6 +16,9 @@

#include <task-dispatcher/common/system_info.hh>

// the size of a task in cachelines (= 64B)
#define TD_FIXED_TASK_SIZE 2
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not really public api, right? and does it have to be a macro? a detail:: constexpr/enum value might be a better fit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, was thinking of making it a CMake exposed option at the time but an enum val is just as good for now

@@ -102,7 +101,7 @@ struct Scheduler::worker_thread_t
// same restrictions as for _resumable_fibers apply (worker_fiber_t::is_waiting_cleaned_up)
resumable_fiber_mpsc_queue pinned_resumable_fibers = {};
// note that this queue uses a spinlock instead of being lock free (TODO)
SpinLock pinned_resumable_fibers_lock = {};
cc::spin_lock pinned_resumable_fibers_lock = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

why the = {}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Redundant

batch < num_batches; //
++batch, start = batch * batch_size, end = cc::min((batch + 1) * batch_size, num_elements))
{
tasks[batch].lambda([=] { func(start, end, batch); });
Copy link
Contributor

Choose a reason for hiding this comment

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

is this by design that the func is copied?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the task is persisted, the original lambda will be dead once it's executed

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.

2 participants