Skip to content

Commit

Permalink
Rewrite the selector table in C++.
Browse files Browse the repository at this point in the history
This replaces a few home-grown datastructures with third-party ones that
get a lot more testing:

 - The home-grown hopscotch hash table is moved to using robin map.  The
   original was designed to be lock free, but we've been using it behind
   a lock for ages.
 - The selector list is now a std::vector.
 - The types list now use std::forward_list.

This also removes a couple of code paths that haven't been used since we
started using the new ABI data structures internally and upgrading at
load time.

The new code tries to differentiate in the static type system between
registered and unregistered selectors.  The check for whether a selector
is registered is fragile and depends on no selector being mapped or
allocated in memory below the total number of selectors.  This check can
now disappear on most code paths.

On a single test machine (not guaranteed to be representative) the test
suite now completes around 20% faster.
  • Loading branch information
davidchisnall committed Feb 19, 2023
1 parent 7c23a07 commit e23882f
Show file tree
Hide file tree
Showing 11 changed files with 836 additions and 751 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
run: |
export LDFLAGS=-L/usr/lib/llvm-${{ matrix.llvm-version }}/lib/
ls -lahR /usr/lib/llvm-${{ matrix.llvm-version }}/lib/
cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{matrix.build-type}} -G Ninja -DCMAKE_C_COMPILER=clang-${{matrix.llvm-version}} -DCMAKE_ASM_COMPILER=clang-${{matrix.llvm-version}} -DCMAKE_CXX_COMPILER=clang++-${{matrix.llvm-version}} -DCMAKE_CXX_FLAGS="-stdlib=${{matrix.cxxlib}}"
cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{matrix.build-type}} -G Ninja -DTESTS=ON -DCMAKE_C_COMPILER=clang-${{matrix.llvm-version}} -DCMAKE_ASM_COMPILER=clang-${{matrix.llvm-version}} -DCMAKE_CXX_COMPILER=clang++-${{matrix.llvm-version}} -DCMAKE_CXX_FLAGS="-stdlib=${{matrix.cxxlib}}"
# Build with a nice ninja status line
- name: Build
working-directory: ${{github.workspace}}/build
Expand Down
6 changes: 3 additions & 3 deletions ANNOUNCE
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ https://github.com/gnustep/libobjc2/archive/v2.2.tar.gz

The submodule is available from:

https://github.com/Tessil/robin-map/archive/757de82.zip
https://github.com/Tessil/robin-map/archive/757de82.tar.gz
https://github.com/Tessil/robin-map/archive/d37a410.zip
https://github.com/Tessil/robin-map/archive/d37a410.tar.gz

This will extract as robin-map-757de829927489bee55ab02147484850c687b620.
This will extract as robin-map-d37a41003bfbc7e12e34601f93c18ca2ff6d7c07.
You must move the contents of that directory into third_party/robin_map in the
libobjc2 tree.

Expand Down
4 changes: 3 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ set(libobjc_C_SRCS
protocol.c
runtime.c
sarray2.c
selector_table.c
sendmsg2.c
)
set(libobjc_HDRS
Expand Down Expand Up @@ -95,6 +94,9 @@ set(libBlocksRuntime_COMPATIBILITY_HDRS
Block.h
Block_private.h
)
set(libobjc_CXX_SRCS
selector_table.cc
)
# Windows does not use DWARF EH
if (WIN32)
list(APPEND libobjc_CXX_SRCS eh_win32_msvc.cc)
Expand Down
2 changes: 1 addition & 1 deletion loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ static void init_runtime(void)
INIT_LOCK(runtime_mutex);
// Create the various tables that the runtime needs.
init_selector_tables();
init_dispatch_tables();
init_protocol_table();
init_class_tables();
init_dispatch_tables();
init_alias_table();
init_arc();
init_trampolines();
Expand Down
33 changes: 33 additions & 0 deletions lock.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,37 @@ extern mutex_t runtime_mutex;
#define UNLOCK_RUNTIME() UNLOCK(&runtime_mutex)
#define LOCK_RUNTIME_FOR_SCOPE() LOCK_FOR_SCOPE(&runtime_mutex)

#ifdef __cplusplus
/**
* C++ wrapper around our mutex, for use with std::lock_guard and friends.
*/
class RecursiveMutex
{
/// The underlying mutex
mutex_t mutex;

public:
/**
* Explicit initialisation of the underlying lock, so that this can be a
* global.
*/
void init()
{
INIT_LOCK(mutex);
}

/// Acquire the lock.
void lock()
{
LOCK(&mutex);
}

/// Release the lock.
void unlock()
{
UNLOCK(&mutex);
}
};
#endif

#endif // __LIBOBJC_LOCK_H_INCLUDED__
45 changes: 45 additions & 0 deletions pool.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#pragma once
#ifdef _WIN32
#include "safewindows.h"
#if defined(WINAPI_FAMILY) && WINAPI_FAMILY != WINAPI_FAMILY_DESKTOP_APP && _WIN32_WINNT >= 0x0A00
// Prefer the *FromApp versions when we're being built in a Windows Store App context on
// Windows >= 10. *FromApp require the application to be manifested for "codeGeneration".
#define VirtualAlloc VirtualAllocFromApp
#define VirtualProtect VirtualProtectFromApp
#endif // App family partition

inline void *allocate_pages(size_t size)
{
return VirtualAlloc(nullptr, size, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE);
}

#else
#include <sys/mman.h>
inline void *allocate_pages(size_t size)
{
void *ret = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
return ret == MAP_FAILED ? nullptr : ret;

}
#endif

template<typename T>
class PoolAllocate
{
static constexpr size_t PageSize = 4096;
static constexpr size_t ChunkSize = sizeof(T) * PageSize;
static inline size_t index = PageSize;
static inline T *buffer = nullptr;
public:
static T *allocate()
{
if (index == PageSize)
{
index = 0;
buffer = static_cast<T*>(allocate_pages(ChunkSize));
}
return &buffer[index++];
}
};


29 changes: 8 additions & 21 deletions selector.h
Original file line number Diff line number Diff line change
@@ -1,21 +1,5 @@
#ifndef OBJC_SELECTOR_H_INCLUDED
#define OBJC_SELECTOR_H_INCLUDED
/**
* Structure used to store the types for a selector. This allows for a quick
* test to see whether a selector is polymorphic and allows enumeration of all
* type encodings for a given selector.
*
* This is the same size as an objc_selector, so we can allocate them from the
* objc_selector pool.
*
* Note: For ABI v10, we can probably do something a bit more sensible here and
* make selectors into a linked list.
*/
struct sel_type_list
{
const char *value;
struct sel_type_list *next;
};

/**
* Structure used to store selectors in the list.
Expand Down Expand Up @@ -60,17 +44,20 @@ static SEL sel_getUntyped(SEL aSel)
return sel_registerTypedName_np(sel_getName(aSel), 0);
}

/**
* Returns whether a selector is mapped.
*/
BOOL isSelRegistered(SEL sel);

#ifdef __cplusplus
extern "C"
{
#endif
/**
* Registers the selector. This selector may be returned later, so it must not
* be freed.
*/
SEL objc_register_selector(SEL aSel);

#ifdef __cplusplus
}
#endif

/**
* SELECTOR() macro to work around the fact that GCC hard-codes the type of
* selectors. This is functionally equivalent to @selector(), but it ensures
Expand Down
Loading

0 comments on commit e23882f

Please sign in to comment.