Skip to content

Commit

Permalink
(#499) Minor cleanup of INVALID_TID, and MAX_THREADS in task-system
Browse files Browse the repository at this point in the history
This commit applies some minor clean-up to task system as a follow-on
to the larger rework done under PR #497. Consistently use STATUS_BUSY,
as a way to report when all concurrent threads are in-use. Minor
changes are done to task system unit-test code & cleanup of comments.
  • Loading branch information
gapisback committed Jan 12, 2023
1 parent 60c7910 commit 84484df
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 10 deletions.
5 changes: 3 additions & 2 deletions src/task.c
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ task_create_thread_with_hooks(platform_thread *thread,
platform_error_log("Cannot create a new thread as the limit on"
" concurrent threads, %d, will be exceeded.\n",
MAX_THREADS);
return STATUS_LIMIT_EXCEEDED;
return STATUS_BUSY;
}

if (0 < scratch_size) {
Expand Down Expand Up @@ -359,8 +359,9 @@ task_register_thread(task_system *ts,
thread_tid);

thread_tid = task_allocate_threadid(ts);
// Unavailable threads is a temporary state that could go away.
if (thread_tid == INVALID_TID) {
return STATUS_NO_SPACE;
return STATUS_BUSY;
}

platform_assert(ts->thread_scratch[thread_tid] == NULL,
Expand Down
18 changes: 10 additions & 8 deletions tests/unit/task_system_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ CTEST2(task_system, test_one_thread_using_extern_apis)
* Background threads are off, by default.
* ------------------------------------------------------------------------
*/
CTEST2(task_system, test_multiple_threads)
CTEST2(task_system, test_max_threads_using_lower_apis)
{
platform_thread new_thread;
thread_config thread_cfg[MAX_THREADS];
Expand Down Expand Up @@ -399,7 +399,7 @@ CTEST2(task_system, test_task_system_creation_with_bg_threads)
/*
* ------------------------------------------------------------------------
* Test creation of task system using up the threads for background threads.
* Verify ththe we can create just one more user-thread and that the next
* Verify that we can create just one more user-thread and that the next
* user-thread creation should fail with a proper error message.
* ------------------------------------------------------------------------
*/
Expand All @@ -410,7 +410,7 @@ CTEST2(task_system, test_use_all_but_one_threads_for_bg_threads)
// Destroy the task system setup by the harness, by default, w/o bg threads.
task_system_destroy(data->hid, &data->tasks);

// Consume all available threads with background threads.
// Consume all-but-one available threads with background threads.
rc = create_task_system_with_bg_threads(data, 1, (MAX_THREADS - 3));
ASSERT_TRUE(SUCCESS(rc));

Expand Down Expand Up @@ -692,6 +692,9 @@ exec_one_of_n_threads(void *arg)
{
thread_config *thread_cfg = (thread_config *)arg;

// Before registration, thread ID should be in an uninit'ed state
ASSERT_EQUAL(INVALID_TID, platform_get_tid());

task_register_this_thread(thread_cfg->tasks, trunk_get_scratch_size());

threadid this_threads_index = platform_get_tid();
Expand All @@ -710,15 +713,14 @@ exec_one_of_n_threads(void *arg)
task_deregister_this_thread(thread_cfg->tasks);

// Register / de-register of thread with SplinterDB's task system is just
// SplinterDB's jugglery to keep track of resources. get_tid() should still
// remain the expected index into the threads[] array.
// SplinterDB's jugglery to keep track of resources. Deregistration should
// have re-init'ed the thread ID.
threadid get_tid_after_deregister = platform_get_tid();
ASSERT_EQUAL(INVALID_TID,
get_tid_after_deregister,
"get_tid_after_deregister=%lu is != the index into"
" thread array, %lu ",
"get_tid_after_deregister=%lu should be an invalid tid, %lu",
get_tid_after_deregister,
this_threads_index);
INVALID_TID);
}

/*
Expand Down

0 comments on commit 84484df

Please sign in to comment.