Static scheduler #241
Static scheduler #241
Conversation
FS: Add reactor local time and remove chain optimization
…nd-clear to sync-advance-clear to advance time at the end of the hyperperiod
Add variables to schedules
…uxiliary EXE (helps a lot).
This reverts commit 3a37b15.
WalkthroughThe changes span multiple core components of the system, with key modifications including the introduction of busy-wait loops in timing functions, new static scheduling logic, and the addition of a circular buffer utility. Enhancements also involve conditional compilation directives for optimization, updates in tracepoint management for better granularity, and relevant infrastructural changes in Changes
Sequence Diagram(s)sequenceDiagram
actor Developer
participant Core Clock as core/clock.c
participant Reactor as core/reactor.c
participant SchedulerAdded as include/core/threaded/scheduler_static_functions.h
Developer->>Core Clock: Call lf_clock_cond_timedwait()
Core Clock->>SchedulerAdded: Busy-wait until wakeup_time
SchedulerAdded-->>Core Clock: Return true
Core Clock-->>Developer: Returns true
Developer->>Reactor: Call wait_until()
Reactor->>Reactor: Busy-wait until wakeup_time
Reactor-->>Developer: Returns 0
sequenceDiagram
actor Developer
participant ReactionCommon as core/reactor_common.c
participant OptimizedScheduler as Scheduler with REACTION_CHAIN_OPTIMIZATION
Developer->>ReactionCommon: Call schedule_output_reactions()
alt REACTION_CHAIN_OPTIMIZATION Defined
ReactionCommon->>OptimizedScheduler: Check downstream reaction conditions
OptimizedScheduler->>OptimizedScheduler: Execute _lf_trigger_reaction for single downstream reaction
else REACTION_CHAIN_OPTIMIZATION Not Defined
ReactionCommon->>OptimizedScheduler: Directly queue reaction via _lf_trigger_reaction
end
OptimizedScheduler-->>Developer: Reaction scheduled
Tip AI model upgrade
|
|
@coderabbitai review |
Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Outside diff range and nitpick comments (13)
include/core/utils/circular_buffer.h (2)
1-2: Include a brief description and author information.Add a brief description and author information to the header file.
+#ifndef CIRCULAR_BUFFER_H +#define CIRCULAR_BUFFER_H +/** + * @file circular_buffer.h + * @brief Circular buffer data structure and function declarations. + * @author Your Name + */
18-24: Add documentation for each function.Provide detailed documentation for each function, including parameters and return values.
+/** + * @brief Initialize the circular buffer. + * @param cb Pointer to the circular buffer. + * @param capacity Maximum number of items in the buffer. + * @param sz Size of each item in the buffer. + */ void cb_init(circular_buffer *cb, size_t capacity, size_t sz); +/** + * @brief Free the circular buffer. + * @param cb Pointer to the circular buffer. + */ void cb_free(circular_buffer *cb); +/** + * @brief Push an item to the back of the circular buffer. + * @param cb Pointer to the circular buffer. + * @param item Pointer to the item to be added. + */ void cb_push_back(circular_buffer *cb, const void *item); +/** + * @brief Pop an item from the front of the circular buffer. + * @param cb Pointer to the circular buffer. + * @param item Pointer to the item to be removed. + */ void cb_pop_front(circular_buffer *cb, void *item); +/** + * @brief Remove an item from the front of the circular buffer. + * @param cb Pointer to the circular buffer. + */ void cb_remove_front(circular_buffer *cb); +/** + * @brief Peek at the item at the front of the circular buffer. + * @param cb Pointer to the circular buffer. + * @return Pointer to the item at the front of the buffer. + */ void* cb_peek(circular_buffer *cb); +/** + * @brief Dump the events in the circular buffer. + * @param cb Pointer to the circular buffer. + */ void cb_dump_events(circular_buffer *cb);include/core/threaded/scheduler_instructions.h (3)
1-4: Include a brief description and author information.Add a brief description and author information to the header file.
+#ifndef SCHEDULER_INSTRUCTIONS_H +#define SCHEDULER_INSTRUCTIONS_H +/** + * @file scheduler_instructions.h + * @brief Format of the instruction set for the PRET VM in C platforms. + * @author Shaokai Lin + */
43-46: Add documentation for theoperand_tunion.Provide detailed documentation for the
operand_tunion, including members and usage.+/** + * @brief A union representing a single operand for the PRET VM. + * @details This union can store either a register pointer or an immediate value. + */ typedef union { reg_t* reg; imm_t imm; } operand_t;
51-60: Add documentation for the function pointer type.Provide detailed documentation for the
function_virtual_instruction_tfunction pointer type, including parameters and return values.+/** + * @brief Virtual instruction function pointer. + * @param scheduler Pointer to the scheduler. + * @param worker_number The worker number. + * @param op1 First operand. + * @param op2 Second operand. + * @param op3 Third operand. + * @param debug Debug flag. + * @param pc Program counter. + * @param returned_reaction Pointer to the returned reaction. + * @param exit_loop Flag to indicate if the loop should exit. + */ typedef void (*function_virtual_instruction_t)( lf_scheduler_t* scheduler, size_t worker_number, operand_t op1, operand_t op2, operand_t op3, bool debug, size_t* pc, reaction_t** returned_reaction, bool* exit_loop);core/utils/circular_buffer.c (3)
36-46: Improve error message incb_push_back.The error message for a full buffer could be more informative by including the buffer's capacity.
void cb_push_back(circular_buffer *cb, const void *item) { if(cb->count == cb->capacity){ - lf_print("ERROR: Buffer is full. Some in-flight events will be overwritten!"); + lf_print("ERROR: Buffer is full. Capacity: %zu. Some in-flight events will be overwritten!", cb->capacity); } memcpy(cb->head, item, cb->sz); cb->head = (char*)cb->head + cb->sz; if(cb->head == cb->buffer_end) cb->head = cb->buffer; cb->count++; }
48-60: Improve error message incb_pop_front.The error message for an empty buffer could be more informative by including the buffer's current count.
void cb_pop_front(circular_buffer *cb, void *item) { if(cb->count == 0){ // handle error - lf_print("ERROR: Popping from an empty buffer!"); + lf_print("ERROR: Popping from an empty buffer! Current count: %zu.", cb->count); return; } memcpy(item, cb->tail, cb->sz); cb->tail = (char*)cb->tail + cb->sz; if(cb->tail == cb->buffer_end) cb->tail = cb->buffer; cb->count--; }
62-73: Improve error message incb_remove_front.The error message for an empty buffer could be more informative by including the buffer's current count.
void cb_remove_front(circular_buffer *cb) { if(cb->count == 0){ // handle error - lf_print("ERROR: Removing from an empty buffer!"); + lf_print("ERROR: Removing from an empty buffer! Current count: %zu.", cb->count); return; } cb->tail = (char*)cb->tail + cb->sz; if(cb->tail == cb->buffer_end) cb->tail = cb->buffer; cb->count--; }core/threaded/scheduler_static.c (2)
3-25: Ensure consistency in copyright notices.The copyright notices for the University of Texas at Dallas and the University of California at Berkeley should be consistent with other files in the repository.
482-484: Remove unused variableopcode.The variable
opcodeis unused in thelf_sched_get_ready_reactionfunction. Removing it will improve code readability and maintainability.- opcode = current_schedule[*pc].opcode; // FIXME: Opcode is unused.util/tracing/trace_to_chrome.c (2)
54-62: Consider removing commented-out code.The commented-out
scaling_factorvariable may be unnecessary. If it is not intended for future use, consider removing it to improve code clarity and maintainability.
80-148: Return a constant string for unrecognized event types.In the default case of the switch statement, consider returning a constant string (e.g.,
const char* unknown = "UNKNOWN"; return unknown;) instead of a literal string to avoid potential issues with string literals.core/reactor_common.c (1)
872-873: Avoid hacky solutions.The comment indicates that the current solution is a temporary fix. Consider refactoring to implement a more robust solution.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (21)
- core/clock.c (1 hunks)
- core/lf_token.c (1 hunks)
- core/reactor.c (1 hunks)
- core/reactor_common.c (2 hunks)
- core/threaded/CMakeLists.txt (1 hunks)
- core/threaded/scheduler_static.c (1 hunks)
- core/trace.c (4 hunks)
- core/utils/CMakeLists.txt (1 hunks)
- core/utils/circular_buffer.c (1 hunks)
- include/api/reaction_macros.h (4 hunks)
- include/core/environment.h (1 hunks)
- include/core/lf_types.h (2 hunks)
- include/core/reactor.h (1 hunks)
- include/core/reactor_common.h (1 hunks)
- include/core/threaded/scheduler_instance.h (4 hunks)
- include/core/threaded/scheduler_instructions.h (1 hunks)
- include/core/threaded/scheduler_static_functions.h (1 hunks)
- include/core/trace.h (5 hunks)
- include/core/utils/circular_buffer.h (1 hunks)
- test/CMakeLists.txt (1 hunks)
- util/tracing/trace_to_chrome.c (6 hunks)
Files not reviewed due to errors (1)
- include/api/reaction_macros.h (no review received)
Files skipped from review due to trivial changes (3)
- core/threaded/CMakeLists.txt
- include/core/reactor.h
- include/core/reactor_common.h
Additional comments not posted (31)
test/CMakeLists.txt (1)
1-1: Verify the definition and usage ofSCHED_STATIC.Ensure that
SCHED_STATICis defined and used correctly in the build process.Verification successful
SCHED_STATIC is correctly defined and used in the codebase.
The macro
SCHED_STATICis defined ininclude/core/lf_types.hand is used in various files for conditional compilation, ensuring its proper integration in the build process.
- Definition:
include/core/lf_types.h- Usage: Multiple files including
include/core/reactor_common.h,include/core/environment.h,include/api/reaction_macros.h,core/trace.c, and others.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition and usage of SCHED_STATIC in the codebase. # Test: Search for the definition of SCHED_STATIC. Expect: A valid definition. rg --type-add 'cmake' --type 'cmake' $'SCHED_STATIC'Length of output: 107
Script:
#!/bin/bash # Description: Verify the definition and usage of SCHED_STATIC in the codebase. # Test: Search for the definition of SCHED_STATIC. Expect: A valid definition. rg 'SCHED_STATIC'Length of output: 1173
core/utils/CMakeLists.txt (1)
1-1: Verify the implementation and integration ofcircular_buffer.c.Ensure that
circular_buffer.cis correctly implemented and integrated into the build process.Verification successful
circular_buffer.cis correctly implemented and integrated into the build process.
- The file
core/utils/circular_buffer.ccontains a valid implementation of a circular buffer.- The file is referenced in
core/utils/CMakeLists.txt, ensuring it is included in the build process.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation and integration of circular_buffer.c in the codebase. # Test: Search for the implementation of circular_buffer.c. Expect: A valid implementation file. fd 'circular_buffer.c' --type fLength of output: 60
Script:
#!/bin/bash # Description: Verify the implementation and integration of circular_buffer.c in the codebase. # Test: Check the implementation of circular_buffer.c echo "Implementation of circular_buffer.c:" cat core/utils/circular_buffer.c # Test: Verify if circular_buffer.c is included in the build process echo "References to circular_buffer.c in CMakeLists.txt:" rg 'circular_buffer.c' --glob '**/CMakeLists.txt'Length of output: 2702
core/clock.c (1)
58-60: Verify the necessity and performance implications of the busy-wait loop.The busy-wait loop might have performance implications. Ensure that it is necessary and does not introduce performance issues.
core/utils/circular_buffer.c (2)
75-80: LGTM!The function correctly handles the case when the buffer is empty.
82-93: LGTM!The function correctly handles the buffer wrap-around and prints the events.
include/core/threaded/scheduler_static_functions.h (12)
15-15: LGTM!The function prototype seems correct.
17-18: LGTM!The function prototype seems correct.
19-20: LGTM!The function prototype seems correct.
21-22: LGTM!The function prototype seems correct.
23-24: LGTM!The function prototype seems correct.
25-26: LGTM!The function prototype seems correct.
27-28: LGTM!The function prototype seems correct.
29-30: LGTM!The function prototype seems correct.
31-32: LGTM!The function prototype seems correct.
33-34: LGTM!The function prototype seems correct.
35-36: LGTM!The function prototype seems correct.
37-46: LGTM!The function prototypes seem correct.
include/core/environment.h (1)
113-120: Verify the usage of new members inenvironment_t.The new members related to static scheduling seem correct. However, ensure that their usage is consistent across the codebase.
include/core/threaded/scheduler_instance.h (2)
48-54: LGTM!The forward declarations and includes for the static scheduler seem correct.
152-190: Verify the usage of new members inlf_scheduler_t.The new members related to static scheduling seem correct. However, ensure that their usage is consistent across the codebase.
Verification successful
Usage of new members in
lf_scheduler_tverified.The new members related to static scheduling have been correctly integrated and are consistently used across the codebase.
pcis used in multiple functions withincore/threaded/scheduler_static.candinclude/core/threaded/scheduler_static_functions.h.static_schedules,reactor_self_instances,num_reactor_self_instances,reaction_instances, andcountersare referenced and utilized appropriately in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of new members in `lf_scheduler_t`. # Test: Search for the new members in the codebase. Expect: Consistent usage. rg --type-add 'c:h' --type-add 'c:cpp' --type-add 'c:cc' --type c 'pc|static_schedules|reactor_self_instances|num_reactor_self_instances|reaction_instances|counters'Length of output: 29444
include/core/lf_types.h (2)
49-56: LGTM!The changes to define
SCHED_STATICand conditionally defineREACTOR_LOCAL_TIMEbased on theSCHEDULERvalue are straightforward and align with the introduction of a static scheduler.
296-302: LGTM!The changes to add members to the
self_base_tstruct for supporting the static scheduler are straightforward and align with the introduction of a static scheduler.include/core/trace.h (5)
79-109: Addition of new trace event types for static scheduler instructions.The new trace event types for static scheduler instructions are well-defined and follow the existing naming conventions.
175-205: Addition of string representations for new trace event types.The string representations for the new static scheduler instructions match the new trace event types and are appropriately named.
255-256: Verify the necessity and impact of increasing the trace buffer capacity.The trace buffer capacity has been increased significantly from 2048 to 131072. Ensure that this change is necessary and that it does not negatively impact memory usage or performance.
500-533: Addition of new tracepoint function declarations for static scheduler instructions.The new tracepoint function declarations for static scheduler instructions are consistent with the existing function naming conventions and parameter types.
640-671: Addition of new tracepoint function definitions for static scheduler instructions when tracing is disabled.The new tracepoint function definitions for static scheduler instructions are consistent with the existing function definitions for disabled tracing.
util/tracing/trace_to_chrome.c (2)
177-183: LGTM!The code correctly handles
static_scheduler_*event types, mapping them to instruction names and formatting them for output.
282-401: LGTM!The code correctly handles
static_scheduler_*event types, setting the appropriatepidandphasevalues for each case.core/trace.c (1)
464-634: LGTM!The new functions for tracing static scheduler instructions are consistent with existing tracing functions and correctly set the event type and invoke the
tracepointfunction with appropriate arguments.core/reactor_common.c (1)
Line range hint
873-905: Ensure correctness of reaction chain optimization.The new conditional compilation directive introduces a new logic branch for reaction chain optimization. Verify that the logic is correct and does not introduce any issues.
Verification successful
Reaction chain optimization logic is correct.
The introduced logic for reaction chain optimization correctly handles the queuing and execution of downstream reactions. The
_lf_trigger_reactionfunction ensures proper queuing, and the conditions for immediate execution based onlast_enabling_reactionare correctly implemented.
core/reactor_common.c: Lines 899-908Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the new reaction chain optimization logic. # Test: Search for the usage of REACTION_CHAIN_OPTIMIZATION. Expect: Correct logic implementation. rg --type c 'REACTION_CHAIN_OPTIMIZATION'Length of output: 169
Script:
#!/bin/bash # Search for the definition and usage of `_lf_trigger_reaction` in the codebase. rg '_lf_trigger_reaction' # Search for the definition and usage of `last_enabling_reaction` in the codebase. rg 'last_enabling_reaction'Length of output: 1841
Script:
#!/bin/bash # Retrieve the implementation of `_lf_trigger_reaction` from core/reactor.c ast-grep --lang c --pattern 'void _lf_trigger_reaction(environment_t* env, reaction_t* reaction, int worker_number) { $$$ }' core/reactor.c # Search for the usage of `last_enabling_reaction` within core/reactor_common.c to understand its context. rg 'last_enabling_reaction' core/reactor_common.cLength of output: 1573
| // FIXME (Shaokai): The function name is probably a misnomer. | ||
| // It should better be _lf_get_new_token. | ||
| // I also don't get why the token isn't simply returned, and | ||
| // _lf_free_token_value or _lf_done_using has to be called. |
There was a problem hiding this comment.
Consider renaming _lf_get_token to _lf_get_new_token and clarify the function's logic.
The comments suggest that the function name _lf_get_token is a misnomer and should be renamed to _lf_get_new_token. Additionally, the logic of calling _lf_free_token_value or _lf_done_using should be clarified.
Would you like assistance in refactoring the function name and clarifying the logic?
core/reactor.c
Outdated
| // return lf_clock_interruptable_sleep_until_locked(env, wakeup_time); | ||
| while (lf_time_physical() < wakeup_time); | ||
| return 0; |
There was a problem hiding this comment.
Avoid using a busy-wait loop.
The busy-wait loop in the wait_until function can lead to high CPU usage. It is recommended to retain the original function call to lf_clock_interruptable_sleep_until_locked or replace it with a more efficient waiting mechanism.
- while (lf_time_physical() < wakeup_time);
+ return lf_clock_interruptable_sleep_until_locked(env, wakeup_time);Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // return lf_clock_interruptable_sleep_until_locked(env, wakeup_time); | |
| while (lf_time_physical() < wakeup_time); | |
| return 0; | |
| return lf_clock_interruptable_sleep_until_locked(env, wakeup_time); |
| // FIXME: Will there be problems if instant_t adds reg_t? | ||
| *dst = *src + op3.imm; |
There was a problem hiding this comment.
Clarify the FIXME comment regarding instant_t and reg_t.
The FIXME comment questions whether there will be problems if instant_t adds reg_t. This should be clarified and addressed to ensure correct functionality.
Would you like assistance in addressing this FIXME comment?
core/threaded/scheduler_static.c
Outdated
| // FIXME: There seems to be an overflow problem. | ||
| // When wakeup_time overflows but lf_time_physical() doesn't, | ||
| // _lf_interruptable_sleep_until_locked() terminates immediately. | ||
| reg_t *src = op1.reg; | ||
| instant_t current_time = lf_time_physical(); | ||
| instant_t wakeup_time = *src + op2.imm; | ||
| instant_t wait_interval = wakeup_time - current_time; | ||
| // LF_PRINT_DEBUG("*** start_time: %lld, wakeup_time: %lld, op1: %lld, op2: %lld, current_physical_time: %lld\n", start_time, wakeup_time, *src, op2.imm, lf_time_physical()); | ||
| LF_PRINT_DEBUG("*** [Line %zu] Worker %zu delaying, current_physical_time: %lld, wakeup_time: %lld, wait_interval: %lld", *pc, worker_number, current_time, wakeup_time, wait_interval); | ||
| if (wait_interval > 0) { | ||
| // if (wait_interval < SPIN_WAIT_THRESHOLD) { | ||
| // // Spin wait if the wait interval is less than 1 ms. | ||
| // while (lf_time_physical() < wakeup_time); | ||
| // } else { | ||
| // // Otherwise sleep. | ||
| // _lf_interruptable_sleep_until_locked(scheduler->env, wakeup_time); | ||
| // } |
There was a problem hiding this comment.
Address the FIXME comment regarding overflow in execute_inst_DU.
The FIXME comment indicates a potential overflow problem when wakeup_time overflows but lf_time_physical() doesn't. This should be addressed to ensure correct functionality.
Would you like assistance in addressing this FIXME comment?
| void *buffer; // data buffer | ||
| void *buffer_end; // end of data buffer | ||
| size_t capacity; // maximum number of items in the buffer | ||
| size_t count; // number of items in the buffer | ||
| size_t sz; // size of each item in the buffer | ||
| void *head; // pointer to head | ||
| void *tail; // pointer to tail |
There was a problem hiding this comment.
Ensure proper memory alignment and handling.
Ensure that the buffer memory is properly aligned and handled to avoid potential issues.
+ void *buffer; // data buffer
+ void *buffer_end; // end of data buffer
+ size_t capacity; // maximum number of items in the buffer
+ size_t count; // number of items in the buffer
+ size_t sz; // size of each item in the buffer
+ void *head; // pointer to head
+ void *tail; // pointer to tail
+ // Ensure proper memory alignment
+ alignas(alignof(max_align_t)) char data[];Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void *buffer; // data buffer | |
| void *buffer_end; // end of data buffer | |
| size_t capacity; // maximum number of items in the buffer | |
| size_t count; // number of items in the buffer | |
| size_t sz; // size of each item in the buffer | |
| void *head; // pointer to head | |
| void *tail; // pointer to tail | |
| void *buffer; // data buffer | |
| void *buffer_end; // end of data buffer | |
| size_t capacity; // maximum number of items in the buffer | |
| size_t count; // number of items in the buffer | |
| size_t sz; // size of each item in the buffer | |
| void *head; // pointer to head | |
| void *tail; // pointer to tail | |
| // Ensure proper memory alignment | |
| alignas(alignof(max_align_t)) char data[]; |
| typedef volatile uint64_t reg_t; | ||
| typedef uint64_t imm_t; |
There was a problem hiding this comment.
Ensure proper type safety and usage.
Ensure that the typedefs for reg_t and imm_t are used correctly and safely.
+typedef volatile uint64_t reg_t;
+typedef uint64_t imm_t;
+// Ensure proper type safety
+static_assert(sizeof(reg_t) == sizeof(imm_t), "reg_t and imm_t must be the same size");Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| typedef volatile uint64_t reg_t; | |
| typedef uint64_t imm_t; | |
| typedef volatile uint64_t reg_t; | |
| typedef uint64_t imm_t; | |
| // Ensure proper type safety | |
| static_assert(sizeof(reg_t) == sizeof(imm_t), "reg_t and imm_t must be the same size"); |
| void cb_free(circular_buffer *cb) | ||
| { | ||
| free(cb->buffer); | ||
| // clear out other fields too, just to be safe | ||
| } |
There was a problem hiding this comment.
Reset circular buffer attributes in cb_free.
To prevent potential use-after-free errors, reset other attributes of the circular buffer after freeing the memory.
void cb_free(circular_buffer *cb)
{
free(cb->buffer);
// clear out other fields too, just to be safe
+ cb->buffer = NULL;
+ cb->buffer_end = NULL;
+ cb->capacity = 0;
+ cb->count = 0;
+ cb->sz = 0;
+ cb->head = NULL;
+ cb->tail = NULL;
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void cb_free(circular_buffer *cb) | |
| { | |
| free(cb->buffer); | |
| // clear out other fields too, just to be safe | |
| } | |
| void cb_free(circular_buffer *cb) | |
| { | |
| free(cb->buffer); | |
| // clear out other fields too, just to be safe | |
| cb->buffer = NULL; | |
| cb->buffer_end = NULL; | |
| cb->capacity = 0; | |
| cb->count = 0; | |
| cb->sz = 0; | |
| cb->head = NULL; | |
| cb->tail = NULL; | |
| } |
| void cb_init(circular_buffer *cb, size_t capacity, size_t sz) | ||
| { | ||
| cb->buffer = malloc(capacity * sz); | ||
| if(cb->buffer == NULL) { | ||
| // handle error | ||
| lf_print("ERROR: Fail to allocate memory to circular buffer."); | ||
| return; | ||
| } | ||
| cb->buffer_end = (char *)cb->buffer + capacity * sz; | ||
| cb->capacity = capacity; | ||
| cb->count = 0; | ||
| cb->sz = sz; | ||
| cb->head = cb->buffer; | ||
| cb->tail = cb->buffer; | ||
| } |
There was a problem hiding this comment.
Improve error handling in cb_init.
If memory allocation fails, the function should free any partially allocated resources and return an error code.
void cb_init(circular_buffer *cb, size_t capacity, size_t sz)
{
cb->buffer = malloc(capacity * sz);
if(cb->buffer == NULL) {
// handle error
lf_print("ERROR: Fail to allocate memory to circular buffer.");
- return;
+ cb->buffer_end = NULL;
+ cb->capacity = 0;
+ cb->count = 0;
+ cb->sz = 0;
+ cb->head = NULL;
+ cb->tail = NULL;
+ return -1;
}
cb->buffer_end = (char *)cb->buffer + capacity * sz;
cb->capacity = capacity;
cb->count = 0;
cb->sz = sz;
cb->head = cb->buffer;
cb->tail = cb->buffer;
+ return 0;
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void cb_init(circular_buffer *cb, size_t capacity, size_t sz) | |
| { | |
| cb->buffer = malloc(capacity * sz); | |
| if(cb->buffer == NULL) { | |
| // handle error | |
| lf_print("ERROR: Fail to allocate memory to circular buffer."); | |
| return; | |
| } | |
| cb->buffer_end = (char *)cb->buffer + capacity * sz; | |
| cb->capacity = capacity; | |
| cb->count = 0; | |
| cb->sz = sz; | |
| cb->head = cb->buffer; | |
| cb->tail = cb->buffer; | |
| } | |
| void cb_init(circular_buffer *cb, size_t capacity, size_t sz) | |
| { | |
| cb->buffer = malloc(capacity * sz); | |
| if(cb->buffer == NULL) { | |
| // handle error | |
| lf_print("ERROR: Fail to allocate memory to circular buffer."); | |
| cb->buffer_end = NULL; | |
| cb->capacity = 0; | |
| cb->count = 0; | |
| cb->sz = 0; | |
| cb->head = NULL; | |
| cb->tail = NULL; | |
| return -1; | |
| } | |
| cb->buffer_end = (char *)cb->buffer + capacity * sz; | |
| cb->capacity = capacity; | |
| cb->count = 0; | |
| cb->sz = sz; | |
| cb->head = cb->buffer; | |
| cb->tail = cb->buffer; | |
| return 0; | |
| } |
…into static-schedule
This is draft PR for tracking progress on a static scheduler corresponding to #1830.
Summary by CodeRabbit
New Features
Improvements
Code Quality
Build System
Performance