-
Notifications
You must be signed in to change notification settings - Fork 128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace SOURCE[]
with dynamic array
#186
base: master
Are you sure you want to change the base?
Conversation
@@ -727,7 +784,7 @@ void global_init() | |||
PH1_IR = malloc(MAX_IR_INSTR * sizeof(ph1_ir_t)); | |||
PH2_IR = malloc(MAX_IR_INSTR * sizeof(ph2_ir_t)); | |||
LABEL_LUT = malloc(MAX_LABEL * sizeof(label_lut_t)); | |||
SOURCE = malloc(MAX_SOURCE); | |||
SOURCE = create_source(MAX_SOURCE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking of choosing an appropriate power of 2 for MAX_SOURCE
. perhaps 2^18 (262144 B, or 256 KiB)
or 2^19 (524288 B, or 512 KiB)
would be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then, we do need scientific ways to calculate.
This commit replace `SOURCE` (in `src/global.c`) with a dynamically allocated array instead of a static string. Including: - Replaced char* SOURCE with a dynamically allocated array using malloc and free. - Updated related logic accordingly. Co-authored-by: Kyle Lin <[email protected]>
@@ -270,6 +270,14 @@ typedef struct { | |||
|
|||
typedef struct basic_block basic_block_t; | |||
|
|||
/* source dynamic array definition*/ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this blank line and refine the comments.
return array; | ||
} | ||
|
||
bool source_expand(source_t *self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename self
to src
for readability.
|
||
void source_release(source_t *self) | ||
{ | ||
if (self) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rewrite as following:
if (!src)
return;
free(src->elements);
free(src);
SOURCE[]
with dynamic array
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read https://cbea.ms/git-commit/ carefully and enforce the rules.
This pull request replace
SOURCE
(insrc/global.c
) with a dynamically allocated array instead of a static string. This change improves memory flexibility.Changes
malloc
/free
.Performance Analysis
Before:
/usr/bin/time -v ./out/shecc ./src/main.c
After:
/usr/bin/time -v ./out/shecc ./src/main.c
The dynamic array version used slightly more memory, likely due to the overhead introduced by dynamic memory allocation.
Summary by Bito
This pull request enhances memory flexibility by replacing the static char* SOURCE with a dynamically allocated array. It introduces a new structure for the dynamic array and updates the lexer and parser for compatibility. While there is a slight increase in memory usage, overall performance is improved.Unit tests added: False
Estimated effort to review (1-5, lower is better): 2