Skip to content

Conversation

@icgmilk
Copy link

@icgmilk icgmilk commented Apr 4, 2025

This pull request replace SOURCE (in src/global.c) with a dynamically allocated array instead of a static string. This change improves memory flexibility.

Changes

  • Replaced char* SOURCE with a dynamically allocated array using malloc/free.
  • Updated related logic accordingly.

Performance Analysis

Before: /usr/bin/time -v ./out/shecc ./src/main.c

 Command being timed: "./out/shecc ./src/main.c"
        User time (seconds): 0.15
        System time (seconds): 0.21
        Percent of CPU this job got: 99%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.36
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 756224
        Average resident set size (kbytes): 0
        Major (requiring I/O) page faults: 0
        Minor (reclaiming a frame) page faults: 189673
        Voluntary context switches: 1
        Involuntary context switches: 2
        Swaps: 0
        File system inputs: 0
        File system outputs: 464
        Socket messages sent: 0
        Socket messages received: 0
        Signals delivered: 0
        Page size (bytes): 4096
        Exit status: 0

After: /usr/bin/time -v ./out/shecc ./src/main.c

Command being timed: "./out/shecc ./src/main.c"
        User time (seconds): 0.16
        System time (seconds): 0.33
        Percent of CPU this job got: 100%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.50
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 759424
        Average resident set size (kbytes): 0
        Major (requiring I/O) page faults: 0
        Minor (reclaiming a frame) page faults: 190433
        Voluntary context switches: 0
        Involuntary context switches: 2
        Swaps: 0
        File system inputs: 0
        File system outputs: 464
        Socket messages sent: 0
        Socket messages received: 0
        Signals delivered: 0
        Page size (bytes): 4096
        Exit status: 0

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 dynamic arrays and updates the lexer, parser, and inliner for compatibility. While there is a slight increase in memory usage, overall performance has improved for handling varying input sizes.

Unit tests added: False

Estimated effort to review (1-5, lower is better): 2

@jserv jserv requested review from ChAoSUnItY, DrXiao and fennecJ April 5, 2025 00:51
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);
Copy link
Collaborator

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?

Copy link
Collaborator

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.

@jserv jserv changed the title Replace char* SOURCE with dynamic array Replace SOURCE[] with dynamic array Apr 7, 2025
Copy link
Collaborator

@jserv jserv left a 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.

Replaces the static string "SOURCE" in "src/global.c" with a
dynamically allocated array using malloc and free.

- Replaced SOURCE[] with a dynamically allocated array.
- Updated related logic accordingly.

Co-authored-by: Kyle Lin <[email protected]>
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Apr 14, 2025
@jserv jserv merged commit 61c9d10 into sysprog21:master Apr 14, 2025
6 checks passed
@jserv
Copy link
Collaborator

jserv commented Apr 14, 2025

Thank @icgmilk for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants