Skip to content

Commit

Permalink
Add a TID to the PAL (#536)
Browse files Browse the repository at this point in the history
Making this part of the PAL allows other platforms to replace with
something more suitable.
  • Loading branch information
mjp41 authored May 31, 2022
1 parent 53d9fd2 commit 9464556
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 28 deletions.
20 changes: 9 additions & 11 deletions src/snmalloc/ds/flaglock.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include "../aal/aal.h"
#include "../pal/pal.h"

#include <atomic>
#include <functional>
Expand All @@ -14,6 +15,8 @@ namespace snmalloc
*/
struct DebugFlagWord
{
using ThreadIdentity = DefaultPal::ThreadIdentity;

/**
* @brief flag
* The underlying atomic field.
Expand All @@ -32,7 +35,7 @@ namespace snmalloc
*/
void set_owner()
{
SNMALLOC_ASSERT(nullptr == owner);
SNMALLOC_ASSERT(ThreadIdentity() == owner);
owner = get_thread_identity();
}

Expand All @@ -43,7 +46,7 @@ namespace snmalloc
void clear_owner()
{
SNMALLOC_ASSERT(get_thread_identity() == owner);
owner = nullptr;
owner = ThreadIdentity();
}

/**
Expand All @@ -56,24 +59,19 @@ namespace snmalloc
}

private:
using ThreadIdentity = int const*;

/**
* @brief owner
* We use a pointer to TLS field as the thread identity.
* std::thread::id can be another solution but it does not
* support `constexpr` initialisation on some platforms.
* We use the Pal to provide the ThreadIdentity.
*/
std::atomic<ThreadIdentity> owner = nullptr;
std::atomic<ThreadIdentity> owner = ThreadIdentity();

/**
* @brief get_thread_identity
* @return The identity of current thread.
*/
inline ThreadIdentity get_thread_identity()
static ThreadIdentity get_thread_identity()
{
static thread_local int SNMALLOC_THREAD_IDENTITY = 0;
return &SNMALLOC_THREAD_IDENTITY;
return DefaultPal::get_tid();
}
};

Expand Down
15 changes: 2 additions & 13 deletions src/snmalloc/pal/pal.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,23 +173,12 @@ namespace snmalloc
DefaultPal::error(msg.get_message());
}

static inline size_t get_tid()
{
static thread_local size_t tid{0};
static std::atomic<size_t> tid_source{1};

if (tid == 0)
{
tid = tid_source++;
}
return tid;
}

template<size_t BufferSize, typename... Args>
inline void message(Args... args)
{
MessageBuilder<BufferSize> msg{std::forward<Args>(args)...};
MessageBuilder<BufferSize> msg_tid{"{}: {}", get_tid(), msg.get_message()};
MessageBuilder<BufferSize> msg_tid{
"{}: {}", DefaultPal::get_tid(), msg.get_message()};
DefaultPal::message(msg_tid.get_message());
}
} // namespace snmalloc
16 changes: 15 additions & 1 deletion src/snmalloc/pal/pal_concept.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,20 @@ namespace snmalloc
noexcept->ConceptSame<void>;
};

/**
* The Pal must provide a thread id for debugging. It should not return
* the default value of ThreadIdentity, as that is used as not an tid in some
* places.
*/
template<typename PAL>
concept ConceptPAL_tid = requires()
{
{
PAL::get_tid()
}
noexcept->ConceptSame<typename PAL::ThreadIdentity>;
};

/**
* Absent any feature flags, the PAL must support a crude primitive allocator
*/
Expand Down Expand Up @@ -138,7 +152,7 @@ namespace snmalloc
template<typename PAL>
concept ConceptPAL = ConceptPAL_static_features<PAL>&&
ConceptPAL_static_sizes<PAL>&& ConceptPAL_error<PAL>&&
ConceptPAL_memops<PAL> &&
ConceptPAL_memops<PAL>&& ConceptPAL_tid<PAL> &&
(!pal_supports<Entropy, PAL> ||
ConceptPAL_get_entropy64<
PAL>)&&(!pal_supports<LowMemoryNotification, PAL> ||
Expand Down
3 changes: 2 additions & 1 deletion src/snmalloc/pal/pal_open_enclave.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include "pal_noalloc.h"
#include "pal_tid_default.h"

#ifdef OPEN_ENCLAVE
extern "C" void* oe_memset_s(void* p, size_t p_size, int c, size_t size);
Expand All @@ -25,7 +26,7 @@ namespace snmalloc

using OpenEnclaveBasePAL = PALNoAlloc<OpenEnclaveErrorHandler>;

class PALOpenEnclave : public OpenEnclaveBasePAL
class PALOpenEnclave : public OpenEnclaveBasePAL, public PalTidDefault
{
public:
/**
Expand Down
4 changes: 3 additions & 1 deletion src/snmalloc/pal/pal_posix.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include "../aal/aal.h"
#include "pal_tid_default.h"
#include "pal_timer_default.h"
#if defined(SNMALLOC_BACKTRACE_HEADER)
# include SNMALLOC_BACKTRACE_HEADER
Expand Down Expand Up @@ -38,7 +39,8 @@ namespace snmalloc
* working when an early-malloc error appears.
*/
template<class OS, auto writev = ::writev, auto fsync = ::fsync>
class PALPOSIX : public PalTimerDefaultImpl<PALPOSIX<OS>>
class PALPOSIX : public PalTimerDefaultImpl<PALPOSIX<OS>>,
public PalTidDefault
{
/**
* Helper class to access the `default_mmap_flags` field of `OS` if one
Expand Down
30 changes: 30 additions & 0 deletions src/snmalloc/pal/pal_tid_default.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#pragma once

#include <atomic>

namespace snmalloc
{
class PalTidDefault
{
public:
using ThreadIdentity = size_t;

/**
* @brief Get the an id for the current thread.
*
* @return the thread id, this should never be the default of
* ThreadIdentity. Callers can assume it is a non-default value.
*/
static inline ThreadIdentity get_tid() noexcept
{
static thread_local size_t tid{0};
static std::atomic<size_t> tid_source{0};

if (tid == 0)
{
tid = ++tid_source;
}
return tid;
}
};
} // namespace snmalloc
4 changes: 3 additions & 1 deletion src/snmalloc/pal/pal_windows.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include "../aal/aal.h"
#include "pal_tid_default.h"
#include "pal_timer_default.h"

#ifdef _WIN32
Expand All @@ -26,7 +27,8 @@

namespace snmalloc
{
class PALWindows : public PalTimerDefaultImpl<PALWindows>
class PALWindows : public PalTimerDefaultImpl<PALWindows>,
public PalTidDefault
{
/**
* A flag indicating that we have tried to register for low-memory
Expand Down

0 comments on commit 9464556

Please sign in to comment.