Skip to content

misc format cleanup #167

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.

Sign up for GitHub

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

Merged
LiquidityC merged 3 commits into from
Apr 11, 2026
Merged

misc format cleanup #167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f0e937b
Formats CMakeLists.txt
LiquidityC Apr 11, 2026
eeb6aad
Format all of the profiler lib
LiquidityC Apr 11, 2026
361a9be
Align consecutive macros via clang-format
LiquidityC Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .clang-format
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,7 @@ SpaceAfterCStyleCast: false
SpacesInParentheses: false
SpacesInContainerLiterals: false
BreakBeforeBinaryOperators: NonAssignment
AlignConsecutiveMacros:
Enabled: true
AcrossEmptyLines: false
AcrossComments: false
14 changes: 7 additions & 7 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,22 +92,22 @@ else()
endif()

if ( MSVC )
MESSAGE ( STATUS "Setting MSVC MT switches")
string (REPLACE
MESSAGE ( STATUS "Setting MSVC MT switches")
string (REPLACE
"/MDd"
"/MTd"
CMAKE_C_FLAGS_DEBUG
${CMAKE_C_FLAGS_DEBUG}
)
string (REPLACE
)
string (REPLACE
"/MDd"
"/MTd"
CMAKE_C_FLAGS_RELEASE
${CMAKE_C_FLAGS_RELEASE}
)
)
elseif ( WIN32 )
set (CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -mconsole")
set (CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -mwindows")
set (CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -mconsole")
set (CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -mwindows")
elseif ( GCC AND NOT MINGW )
set (CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -fsanitize=address -fno-omit-frame-pointer")
set (CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -O2")
Expand Down
25 changes: 25 additions & 0 deletions lib/profiler/.clang-format
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
BasedOnStyle: LLVM
IndentWidth: 8
TabWidth: 8
UseTab: ForIndentation
BreakBeforeBraces: Linux
ColumnLimit: 120
PointerAlignment: Right
IndentCaseLabels: true
SpaceBeforeParens: ControlStatements
AlignAfterOpenBracket: Align
AllowShortFunctionsOnASingleLine: None
AllowShortIfStatementsOnASingleLine: Never
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterReturnType: AllDefinitions
SortIncludes: Never
IndentPPDirectives: None
SpaceAfterCStyleCast: false
SpacesInParentheses: false
SpacesInContainerLiterals: false
BreakBeforeBinaryOperators: NonAssignment
AlignConsecutiveMacros:
Enabled: true
AcrossEmptyLines: false
AcrossComments: false
19 changes: 9 additions & 10 deletions lib/profiler/include/internal/common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
#ifndef COMMON_H_
#define COMMON_H_

#define C_RED "\x1b[31m"
#define C_GREEN "\x1b[32m"
#define C_YELLOW "\x1b[33m"
#define C_BLUE "\x1b[34m"
#define C_MAGENTA "\x1b[35m"
#define C_CYAN "\x1b[36m"
#define C_WHITE "\x1b[37m"
#define C_RESET "\x1b[0m"

#endif // COMMON_H_
#define C_RED "\x1b[31m"
#define C_GREEN "\x1b[32m"
#define C_YELLOW "\x1b[33m"
#define C_BLUE "\x1b[34m"
#define C_MAGENTA "\x1b[35m"
#define C_CYAN "\x1b[36m"
#define C_WHITE "\x1b[37m"
#define C_RESET "\x1b[0m"

#endif // COMMON_H_
27 changes: 13 additions & 14 deletions lib/profiler/include/internal/profiler_c.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,25 @@
#define ANCHOR_CAPACITY 4096

typedef struct ProfileAnchor {
const char *label;
size_t hits;
uint64_t elapsed_exclusive;
uint64_t elapsed_inclusive;
size_t processed_byte_count;
const char *label;
size_t hits;
uint64_t elapsed_exclusive;
uint64_t elapsed_inclusive;
size_t processed_byte_count;
} ProfileAnchor;

typedef struct Profiler {
uint64_t start, stop;
size_t len;
ProfileAnchor anchors[ANCHOR_CAPACITY];
uint64_t start, stop;
size_t len;
ProfileAnchor anchors[ANCHOR_CAPACITY];
} Profiler;

typedef struct AnchorBlock {
const char *label;
uint64_t start;
uint64_t old_elapsed_inclusive;
size_t processed_byte_count;
char *parent_anchor;
const char *label;
uint64_t start;
uint64_t old_elapsed_inclusive;
size_t processed_byte_count;
char *parent_anchor;
} AnchorBlock;

void prof_init(void);
Expand All @@ -43,4 +43,3 @@ AnchorBlock make_anchor_block(const char *label, size_t used_bytes);
void read_anchor_block(const AnchorBlock *anchor);

#endif // _STOP_CLOCK_H_

8 changes: 4 additions & 4 deletions lib/profiler/include/macros.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,22 @@
prof_print(fp);

#define _NameConcat(A, B) A##B
#define NameConcat(A, B) _NameConcat(A, B)
#define NameConcat(A, B) _NameConcat(A, B)

#define TIME_BANDWIDTH_BEGIN(label, bytes) AnchorBlock NameConcat(Block, label) = make_anchor_block(#label, bytes)
#define TIME_BANDWIDTH_END(label) read_anchor_block(&Block##label)
#define TIME_BANDWIDTH_END(label) read_anchor_block(&Block##label)
#define TIME_BANDWIDTH(label, bytes) \
AnchorBlock NameConcat(Block, label) __attribute__((cleanup(read_anchor_block))); \
NameConcat(Block, label) = make_anchor_block(#label, bytes)

#define TIME_BLOCK_BEGIN(label) TIME_BANDWIDTH_BEGIN(label, 0)
#define TIME_BLOCK_END(label) TIME_BANDWIDTH_END(label)
#define TIME_BLOCK_END(label) TIME_BANDWIDTH_END(label)
#define TIME_BLOCK(label) \
AnchorBlock NameConcat(Block, label) __attribute__((cleanup(read_anchor_block))); \
NameConcat(Block, label) = make_anchor_block(#label, 0)

#define TIME_FUNC_BEGIN() AnchorBlock NameConcat(Block, __func__) = make_anchor_block(__func__, 0)
#define TIME_FUNC_END() read_anchor_block(&Block##__func__)
#define TIME_FUNC_END() read_anchor_block(&Block##__func__)
#define TIME_FUNC() \
AnchorBlock NameConcat(Block, __func__) __attribute__((cleanup(read_anchor_block))); \
NameConcat(Block, __func__) = make_anchor_block(__func__, 0)
Expand Down
3 changes: 1 addition & 2 deletions lib/profiler/include/perf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,4 @@ uint64_t perf_read_page_fault_count(void);

void perf_close(void);

#endif // _PERF_H_

#endif // _PERF_H_
1 change: 0 additions & 1 deletion lib/profiler/include/profiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,3 @@ extern "C" {
#endif

#endif // _PROFILER_H_

1 change: 0 additions & 1 deletion lib/profiler/include/rdtsc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,3 @@ uint64_t estimate_cpu_freq(uint64_t test_time);
#endif

#endif // _RDTSC_H_

67 changes: 30 additions & 37 deletions lib/profiler/include/repetition_tester.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,54 +5,48 @@
#include <stdbool.h>
#include <stdio.h>

#define rept_error(tester, msg) \
do { \
fprintf(stderr, "%s()[%d] %s\n", __func__, __LINE__, msg); \
tester->mode = TestMode_Error; \
} while (0)

typedef enum TestMode {
TestMode_Uninitialized,
TestMode_Testing,
TestMode_Completed,
TestMode_Error
} TestMode;
#define rept_error(tester, msg) \
do { \
fprintf(stderr, "%s()[%d] %s\n", __func__, __LINE__, msg); \
tester->mode = TestMode_Error; \
} while (0)

typedef enum TestMode { TestMode_Uninitialized, TestMode_Testing, TestMode_Completed, TestMode_Error } TestMode;

typedef enum RepetitionValueType {
RepVal_test_count,
RepVal_page_faults,
RepVal_byte_count,
RepVal_cpu_time,
RepVal_count
RepVal_test_count,
RepVal_page_faults,
RepVal_byte_count,
RepVal_cpu_time,
RepVal_count
} RepetitionValueType;

typedef struct RepetitionValue {
uint64_t E[RepVal_count];
uint64_t E[RepVal_count];
} RepetitionValue;

typedef struct RepetitionTestResult {
RepetitionValue total;
RepetitionValue min;
RepetitionValue max;
RepetitionValue total;
RepetitionValue min;
RepetitionValue max;
} RepetitionTestResult;

typedef struct RepetitionTester {
uint64_t target_processed_byte_count;
uint64_t cpu_timer_freq;
uint64_t test_time;
uint64_t start_time;

TestMode mode;
bool print_new_mins;
uint32_t open_block_count;
uint32_t close_block_count;

RepetitionValue current;
RepetitionTestResult results;
uint64_t target_processed_byte_count;
uint64_t cpu_timer_freq;
uint64_t test_time;
uint64_t start_time;

TestMode mode;
bool print_new_mins;
uint32_t open_block_count;
uint32_t close_block_count;

RepetitionValue current;
RepetitionTestResult results;
} RepetitionTester;

void rept_setup(RepetitionTester *tester, uint64_t target_bytes,
uint64_t cpu_timer_freq, uint32_t test_time);
void rept_setup(RepetitionTester *tester, uint64_t target_bytes, uint64_t cpu_timer_freq, uint32_t test_time);

@coderabbitai coderabbitai Bot Apr 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Revert parameter name to match implementation and preserve semantic clarity.

The parameter name was changed from test_time_seconds to test_time, creating a mismatch with the implementation which still uses test_time_seconds. More importantly, this removes critical semantic information: the parameter represents a duration in seconds, while the struct member test_time (line 37) represents CPU timer ticks. The implementation converts between them at line 34 of the source file.

Renaming this parameter:

  1. Creates a header/implementation inconsistency
  2. Removes the unit information from the API
  3. Could confuse users who might conflate the parameter with the struct member
🔧 Proposed fix 
-void rept_setup(RepetitionTester *tester, uint64_t target_bytes, uint64_t cpu_timer_freq, uint32_t test_time);
+void rept_setup(RepetitionTester *tester, uint64_t target_bytes, uint64_t cpu_timer_freq, uint32_t test_time_seconds);
📝 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.

Suggested change
void rept_setup(RepetitionTester *tester, uint64_t target_bytes, uint64_t cpu_timer_freq, uint32_t test_time);
void rept_setup(RepetitionTester *tester, uint64_t target_bytes, uint64_t cpu_timer_freq, uint32_t test_time_seconds);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lib/profiler/include/repetition_tester.h` at line 49, The declaration of
rept_setup in the header was changed to use parameter name test_time which
mismatches the implementation and loses unit semantics; revert the parameter
name back to test_time_seconds in the prototype for rept_setup and ensure it
matches the implementation, so the function signature
(rept_setup(RepetitionTester *tester, uint64_t target_bytes, uint64_t
cpu_timer_freq, uint32_t test_time_seconds)) aligns with the conversion logic
that distinguishes the caller-provided seconds from the RepetitionTester struct
member test_time (which is CPU timer ticks).


void rept_begin(RepetitionTester *tester);

Expand All @@ -64,5 +58,4 @@ bool rept_is_testing(RepetitionTester *tester);

void rept_print_results(RepetitionTester *tester, FILE *fp);

#endif // REPETITION_TESTER_H_

#endif // REPETITION_TESTER_H_
41 changes: 21 additions & 20 deletions lib/profiler/src/calc_cpu_freq.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,28 @@
typedef double f64;
typedef uint64_t u64;

int main(void)
int
main(void)
{
u64 os_freq = get_os_time_freq();
u64 cpu_start = read_cpu_timer();
u64 os_start = read_os_timer();
u64 os_elapsed = 0;
u64 os_wait_time = os_freq;
while (os_elapsed < os_wait_time) {
u64 os_end = read_os_timer();
os_elapsed = os_end - os_start;
}
u64 os_freq = get_os_time_freq();
u64 cpu_start = read_cpu_timer();
u64 os_start = read_os_timer();
u64 os_elapsed = 0;
u64 os_wait_time = os_freq;
while (os_elapsed < os_wait_time) {
u64 os_end = read_os_timer();
os_elapsed = os_end - os_start;
}

u64 cpu_end = read_cpu_timer();
u64 cpu_elapsed = cpu_end - cpu_start;
u64 cpu_freq = 0;
if (os_elapsed) {
cpu_freq = (u64)((double) os_freq * cpu_elapsed / os_elapsed);
}
u64 cpu_end = read_cpu_timer();
u64 cpu_elapsed = cpu_end - cpu_start;
u64 cpu_freq = 0;
if (os_elapsed) {
cpu_freq = (u64)((double)os_freq * cpu_elapsed / os_elapsed);
}

printf(" OS Freq: %lu (reported)\n", os_freq);
printf(" OS Seconds: %.4f\n", (f64) os_elapsed/ (f64) os_freq);
printf(" CPU Timer: %lu -> %lu = %lu\n", cpu_start, cpu_end, cpu_elapsed);
printf(" CPU Freq: %lu (guessed)\n", cpu_freq);
printf(" OS Freq: %lu (reported)\n", os_freq);
printf(" OS Seconds: %.4f\n", (f64)os_elapsed / (f64)os_freq);
printf(" CPU Timer: %lu -> %lu = %lu\n", cpu_start, cpu_end, cpu_elapsed);
printf(" CPU Freq: %lu (guessed)\n", cpu_freq);
}
Loading
Loading