Skip to content

Commit

Permalink
Custom Thread StackSize API (#264)
Browse files Browse the repository at this point in the history
* Add API for specifying thread stack size (#243)

* Add API for specifying thread stack size

* Add compile time default stack size

* compile time definitions in cmake

* add local variable for stack size in API

* Adding temporary CMake Debug message

* Removed debug message in CMakeLists.txt, added CMAKE flag to readme

* Reset global variable before running new thread test

* Remove duplicate code and unused variables

* explicit cast

* missing )

* enforce pthread min stack size

* Change name of variable to have *_BYTES for readability

* Update variable name to include 'bytes' by request

* Addressing nit picks

* bound rand stack size value to not exceed max

* Remove rand() test on an OS wrapper API

* Wake up github

* Comment

* Remove lower bound checking for pthread

* Clang

* StackSize and ConstrainedDevice incompatible; Additional unit tests for the thread create API; Add new CI job and script to check CMake flag compatibility

* Add second case

* Fix typo

* Rename variables in the test

* Address comments and add log line in case of conflict again

---------

Co-authored-by: jdelapla <[email protected]>
  • Loading branch information
sirknightj and jdelapla authored Nov 7, 2024
1 parent c97ddb6 commit 007f19c
Show file tree
Hide file tree
Showing 7 changed files with 288 additions and 17 deletions.
11 changes: 11 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,17 @@ jobs:
cmake .. ${{ matrix.config.cmake_flags }}
make -j$(nproc)
cmake-flags-test:
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Run CMake Flag Combination Test
run: |
./tst/scripts/test_cmake_flags.sh >> $GITHUB_STEP_SUMMARY
windows-msvc:
runs-on: windows-2022
env:
Expand Down
10 changes: 10 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ function(enableSanitizer SANITIZER)
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=${SANITIZER}" PARENT_SCOPE)
endfunction()

if(KVS_DEFAULT_STACK_SIZE AND CONSTRAINED_DEVICE)
message(FATAL_ERROR "Conflicting parameters: KVS_DEFAULT_STACK_SIZE and CONSTRAINED_DEVICE cannot both be set.")
elseif(KVS_DEFAULT_STACK_SIZE)
message(STATUS "Building with default stack size: ${KVS_DEFAULT_STACK_SIZE} bytes")
add_definitions(-DKVS_DEFAULT_STACK_SIZE_BYTES=${KVS_DEFAULT_STACK_SIZE})
elseif(CONSTRAINED_DEVICE)
message(STATUS "Building for constrained device with stack size set to 512 KiB (0.5 MiB)")
add_definitions(-DCONSTRAINED_DEVICE)
endif()

if(ADDRESS_SANITIZER)
enableSanitizer("address")
endif()
Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ You can pass the following options to `cmake ..`
* `-DMEMORY_SANITIZER` -- Build with MemorySanitizer
* `-DTHREAD_SANITIZER` -- Build with ThreadSanitizer
* `-DUNDEFINED_BEHAVIOR_SANITIZER` Build with UndefinedBehaviorSanitizer
* `-DBUILD_DEBUG_HEAP` Build debug heap with guard bands and validation. This is ONLY intended for low-level debugging purposes. Default is OFF
* `-DBUILD_DEBUG_HEAP` Build debug heap with guard bands and validation. This is ONLY intended for low-level debugging purposes. Default is OFF.
* `-DALIGNED_MEMORY_MODEL` Build for aligned memory model only devices. Default is OFF.
* `-DCONSTRAINED_DEVICE` -- Sets the default stack size of threads to 512 KiB (0.5 MiB). Incompatible with `-DKVS_DEFAULT_STACK_SIZE`. Not available in Windows. Default is OFF.
* `-DKVS_DEFAULT_STACK_SIZE` -- Value in bytes for default stack size of threads. Incompatible with `-DCONSTRAINED_DEVICE`. Default is system default.

### Build
To build the library run make in the build directory you executed CMake.
Expand Down
16 changes: 10 additions & 6 deletions src/common/include/com/amazonaws/kinesis/video/common/CommonDefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,7 @@ typedef VOID (*unlockMutex)(MUTEX);
typedef BOOL (*tryLockMutex)(MUTEX);
typedef VOID (*freeMutex)(MUTEX);
typedef STATUS (*createThread)(PTID, startRoutine, PVOID);
typedef STATUS (*createThreadWithParams)(PTID, startRoutine, SIZE_T, PVOID);
typedef STATUS (*joinThread)(TID, PVOID*);
typedef VOID (*threadSleep)(UINT64);
typedef VOID (*threadSleepUntil)(UINT64);
Expand Down Expand Up @@ -763,6 +764,7 @@ extern unlockMutex globalUnlockMutex;
extern tryLockMutex globalTryLockMutex;
extern freeMutex globalFreeMutex;
extern createThread globalCreateThread;
extern createThreadWithParams globalCreateThreadWithParams;
extern joinThread globalJoinThread;
extern threadSleep globalThreadSleep;
extern threadSleepUntil globalThreadSleepUntil;
Expand Down Expand Up @@ -1043,12 +1045,14 @@ extern PUBLIC_API atomicXor globalAtomicXor;
//
// Thread functionality
//
#define THREAD_CREATE globalCreateThread
#define THREAD_JOIN globalJoinThread
#define THREAD_SLEEP globalThreadSleep
#define THREAD_SLEEP_UNTIL globalThreadSleepUntil
#define THREAD_CANCEL globalCancelThread
#define THREAD_DETACH globalDetachThread
// Wrappers around OS specific utilities for threads. Takes arguments as given.
#define THREAD_CREATE globalCreateThread
#define THREAD_CREATE_WITH_PARAMS globalCreateThreadWithParams
#define THREAD_JOIN globalJoinThread
#define THREAD_SLEEP globalThreadSleep
#define THREAD_SLEEP_UNTIL globalThreadSleepUntil
#define THREAD_CANCEL globalCancelThread
#define THREAD_DETACH globalDetachThread

//
// Static initializers
Expand Down
57 changes: 47 additions & 10 deletions src/utils/src/Thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ PUBLIC_API DWORD WINAPI startWrapperRoutine(LPVOID data)
return retVal;
}

PUBLIC_API STATUS defaultCreateThread(PTID pThreadId, startRoutine start, PVOID args)
PUBLIC_API STATUS defaultCreateThreadWithParams(PTID pThreadId, startRoutine start, SIZE_T stackSize, PVOID args)
{
STATUS retStatus = STATUS_SUCCESS;
HANDLE threadHandle;
Expand All @@ -51,7 +51,7 @@ PUBLIC_API STATUS defaultCreateThread(PTID pThreadId, startRoutine start, PVOID
pWrapper->storedArgs = args;
pWrapper->storedStartRoutine = start;

threadHandle = CreateThread(NULL, 0, startWrapperRoutine, pWrapper, 0, NULL);
threadHandle = CreateThread(NULL, stackSize, startWrapperRoutine, pWrapper, 0, NULL);
CHK(threadHandle != NULL, STATUS_CREATE_THREAD_FAILED);

*pThreadId = (TID) threadHandle;
Expand All @@ -64,6 +64,21 @@ PUBLIC_API STATUS defaultCreateThread(PTID pThreadId, startRoutine start, PVOID
return retStatus;
}

PUBLIC_API STATUS defaultCreateThread(PTID pThreadId, startRoutine start, PVOID args)
{
STATUS retStatus = STATUS_SUCCESS;

#if defined(KVS_DEFAULT_STACK_SIZE_BYTES)
CHK_STATUS(defaultCreateThreadWithParams(pThreadId, start, (SIZE_T) KVS_DEFAULT_STACK_SIZE_BYTES, args));
#else
CHK_STATUS(defaultCreateThreadWithParams(pThreadId, start, 0, args));
#endif

CleanUp:

return retStatus;
}

PUBLIC_API STATUS defaultJoinThread(TID threadId, PVOID* retVal)
{
STATUS retStatus = STATUS_SUCCESS;
Expand Down Expand Up @@ -150,7 +165,7 @@ PUBLIC_API TID defaultGetThreadId()
return (TID) pthread_self();
}

PUBLIC_API STATUS defaultCreateThread(PTID pThreadId, startRoutine start, PVOID args)
PUBLIC_API STATUS defaultCreateThreadWithParams(PTID pThreadId, startRoutine start, SIZE_T stackSize, PVOID args)
{
STATUS retStatus = STATUS_SUCCESS;
pthread_t threadId;
Expand All @@ -159,14 +174,14 @@ PUBLIC_API STATUS defaultCreateThread(PTID pThreadId, startRoutine start, PVOID

CHK(pThreadId != NULL, STATUS_NULL_ARG);

#ifdef CONSTRAINED_DEVICE
pthread_attr_t attr;
pAttr = &attr;
result = pthread_attr_init(pAttr);
CHK_ERR(result == 0, STATUS_THREAD_ATTR_INIT_FAILED, "pthread_attr_init failed with %d", result);
result = pthread_attr_setstacksize(&attr, THREAD_STACK_SIZE_ON_CONSTRAINED_DEVICE);
CHK_ERR(result == 0, STATUS_THREAD_ATTR_SET_STACK_SIZE_FAILED, "pthread_attr_setstacksize failed with %d", result);
#endif
if (stackSize != 0) {
pAttr = &attr;
result = pthread_attr_init(pAttr);
CHK_ERR(result == 0, STATUS_THREAD_ATTR_INIT_FAILED, "pthread_attr_init failed with %d", result);
result = pthread_attr_setstacksize(&attr, stackSize);
CHK_ERR(result == 0, STATUS_THREAD_ATTR_SET_STACK_SIZE_FAILED, "pthread_attr_setstacksize failed with %d", result);
}

result = pthread_create(&threadId, pAttr, start, args);
switch (result) {
Expand Down Expand Up @@ -198,6 +213,27 @@ PUBLIC_API STATUS defaultCreateThread(PTID pThreadId, startRoutine start, PVOID
return retStatus;
}

PUBLIC_API STATUS defaultCreateThread(PTID pThreadId, startRoutine start, PVOID args)
{
STATUS retStatus = STATUS_SUCCESS;

#if defined(KVS_DEFAULT_STACK_SIZE_BYTES) && defined(CONSTRAINED_DEVICE)
DLOGW("KVS_DEFAULT_STACK_SIZE_BYTES and CONSTRAINED_DEVICE are both defined. KVS_DEFAULT_STACK_SIZE_BYTES will take priority.");
#endif

#if defined(KVS_DEFAULT_STACK_SIZE_BYTES)
CHK_STATUS(defaultCreateThreadWithParams(pThreadId, start, (SIZE_T) KVS_DEFAULT_STACK_SIZE_BYTES, args));
#elif defined(CONSTRAINED_DEVICE)
CHK_STATUS(defaultCreateThreadWithParams(pThreadId, start, (SIZE_T) THREAD_STACK_SIZE_ON_CONSTRAINED_DEVICE, args));
#else
CHK_STATUS(defaultCreateThreadWithParams(pThreadId, start, 0, args));
#endif

CleanUp:

return retStatus;
}

PUBLIC_API STATUS defaultJoinThread(TID threadId, PVOID* retVal)
{
STATUS retStatus = STATUS_SUCCESS;
Expand Down Expand Up @@ -330,6 +366,7 @@ PUBLIC_API VOID defaultThreadSleepUntil(UINT64 time)
getTId globalGetThreadId = defaultGetThreadId;
getTName globalGetThreadName = defaultGetThreadName;
createThread globalCreateThread = defaultCreateThread;
createThreadWithParams globalCreateThreadWithParams = defaultCreateThreadWithParams;
threadSleep globalThreadSleep = defaultThreadSleep;
threadSleepUntil globalThreadSleepUntil = defaultThreadSleepUntil;
joinThread globalJoinThread = defaultJoinThread;
Expand Down
65 changes: 65 additions & 0 deletions tst/scripts/test_cmake_flags.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#!/bin/bash

# Define flag combinations as tuples with expected results
# Format: "flags expected_result"
# - expected_result is either OK or FAIL
COMBINATIONS=(
"-DKVS_DEFAULT_STACK_SIZE=65536 -DCONSTRAINED_DEVICE=ON FAIL"
"-DKVS_DEFAULT_STACK_SIZE=65536 OK"
"-DCONSTRAINED_DEVICE=ON OK"
"-DKVS_DEFAULT_STACK_SIZE=65536 -DCONSTRAINED_DEVICE=OFF OK"
"-DKVS_DEFAULT_STACK_SIZE=0 -DCONSTRAINED_DEVICE=ON OK"
)

# Initialize result counters
TOTAL_TESTS=${#COMBINATIONS[@]}
PASSED=0
FAILED=0

# Markdown summary header
SUMMARY="# CMake Flag Test Summary\n"

# Test each combination
for combination in "${COMBINATIONS[@]}"; do
# Extract flags and expected result from the tuple
FLAGS="${combination% *}" # Everything except the last word
EXPECTED_RESULT="${combination##* }" # The last word

# Clean up the build directory before each test
rm -rf build
mkdir build

# Run only the CMake configuration with the current combination
cmake -B build $FLAGS > /dev/null 2>&1
EXIT_CODE=$?

# Check if the exit code matches the expected result
if [[ "$EXPECTED_RESULT" == "FAIL" ]]; then
if [[ $EXIT_CODE -ne 0 ]]; then
PASSED=$((PASSED + 1))
SUMMARY+="* ✅ \`$FLAGS\`: Correctly failed.\n"
else
FAILED=$((FAILED + 1))
SUMMARY+="* ❌ \`$FLAGS\`: Expected failure but succeeded.\n"
fi
else
if [[ $EXIT_CODE -ne 0 ]]; then
FAILED=$((FAILED + 1))
SUMMARY+="* ❌ \`$FLAGS\`: Unexpected failure.\n"
else
PASSED=$((PASSED + 1))
SUMMARY+="* ✅ \`$FLAGS\`: Success as expected.\n"
fi
fi
done

# Generate a Markdown summary
SUMMARY+="\n### Total Tests: $TOTAL_TESTS\n"
SUMMARY+="### Passed: $PASSED\n"
SUMMARY+="### Failed: $FAILED\n"

echo -e "$SUMMARY"

if [[ $FAILED -gt 0 ]]; then
exit 1
fi
Loading

0 comments on commit 007f19c

Please sign in to comment.