Profiler rewview fixes

[LiquidityC]

Summary by CodeRabbit

• Documentation

  • Rewrote profiler guide to focus on profiling, with updated, simpler macro-based examples and refreshed output sample.

• Bug Fixes

  • Improved robustness of performance counter setup and reads.
  • Added safer memory allocation checks to prevent crashes.
  • Increased accuracy of CPU frequency calculation.
  • Moved profiling finalization to app shutdown for more reliable cleanup.

• Refactor

  • Aligned profiler API signatures for consistency across build modes.
  • Minor internal cleanups and initialization improvements for stability.

[coderabbitai]

Walkthrough

This PR updates profiler documentation, aligns a macro signature, tweaks macro semicolon behavior, improves robustness in perf and allocation paths, adjusts CPU frequency math to floating point, cleans up rdtsc includes/defines, and moves profiler shutdown from close() into main() after PHYSFS_deinit().

Changes

Cohort / File(s)	Summary
Docs: Profiler README lib/profiler/README.md	Rewrite from benchmark-centric to profiler-centric; examples switched from StopClock API to macro-based profiling; updated sample output; trimmed API/how-it-works sections.
Public headers/macros alignment lib/profiler/include/macros.h, lib/profiler/include/repetition_tester.h	TIME_BANDWIDTH_BEGIN non-profiling branch now takes (label, bytes); rept_error macro no longer appends a trailing semicolon in expansion.
Profiler internals: safety/cleanup lib/profiler/src/perf.c, lib/profiler/src/rep_return_data.c, lib/profiler/src/rdtsc.c	perf: initialize FD, close/reset on re-setup, validate read size; rep_return_data: NULL-check malloc; rdtsc: remove PROFILER define, add <stdint.h> include.
CPU frequency calculation lib/profiler/src/calc_cpu_freq.c	Switch integer division to double-precision division and cast back for non-zero OS elapsed time.
App shutdown flow src/main.c	Move profiling finalization (PROFILER_STOP and file close) from close() to end of main(), after PHYSFS_deinit().

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Main as main()
  participant PHYS as PHYSFS
  participant Prof as Profiler
  participant File as Output File

  Main->>PHYS: PHYSFS_deinit()
  note over PHYS: Library shutdown

  Main->>Prof: PROFILER_STOP
  note over Prof: Finalize profiling, flush data

  Prof-->>File: write summary
  File-->>Prof: flushed

  Main->>File: fclose()
  note right of Main: Now done at end of main()

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I thump my paws at timing’s end,
Blocks and bytes now neatly penned.
From main I wave a tidy stop,
With safer reads and malloc pop.
Clocks float by in double streams—
Profiler hums, fulfilling dreams. 🥕⏱️

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check	❓ Inconclusive	The title “Profiler rewview fixes” is related to the profiler changes but is too generic, contains a typo, and does not summarize the primary modifications in the pull request.	Please update the title to a concise, correctly spelled sentence that highlights the main change, such as “Refactor profiler README and update timing macros.”

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch add_profiler

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

lib/profiler/README.md (1)

14-33: Replace hard tabs with spaces in code example.

The code example contains hard tabs which should be replaced with spaces for consistency with Markdown formatting standards.

Apply this diff to replace hard tabs with spaces:

 static void random_func(void)
 {
-	TIME_FUNC_BEGIN();
+    TIME_FUNC_BEGIN();
 
-	/* Your code goes here... */
+    /* Your code goes here... */
 
-	TIME_FUNC_END();
+    TIME_FUNC_END();
 }
 
 int main(int argc, char **argv)
 {
-	PROFILER_SETUP();
+    PROFILER_SETUP();
     (void) argc;
     (void) argv;
 
-	random_func();
+    random_func();
 
-	TIME_BLOCK_BEGIN(block_name);
-	/* Your code goes here */
-	TIME_BLOCK_END(block_name);
+    TIME_BLOCK_BEGIN(block_name);
+    /* Your code goes here */
+    TIME_BLOCK_END(block_name);
 
-	PROFILER_STOP(stdout);
+    PROFILER_STOP(stdout);
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5da2144 and a70c317.

📒 Files selected for processing (8)

• lib/profiler/README.md (1 hunks)
• lib/profiler/include/macros.h (1 hunks)
• lib/profiler/include/repetition_tester.h (1 hunks)
• lib/profiler/src/calc_cpu_freq.c (1 hunks)
• lib/profiler/src/perf.c (3 hunks)
• lib/profiler/src/rdtsc.c (1 hunks)
• lib/profiler/src/rep_return_data.c (1 hunks)
• src/main.c (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

lib/profiler/src/rep_return_data.c (2)

src/util.c (2)

• ec_malloc (132-140)
• ec_realloc (152-159)

test/test_util.c (1)

• test_util_ec_malloc (21-28)

🪛 markdownlint-cli2 (0.18.1)

lib/profiler/README.md

14-14: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

16-16: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

18-18: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

23-23: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

27-27: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

29-29: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

30-30: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

31-31: Hard tabs
Column: 1

(MD010, no-hard-tabs)

---

33-33: Hard tabs
Column: 1

(MD010, no-hard-tabs)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: Mac (GCC)
• GitHub Check: Ubuntu (mingw)
• GitHub Check: Mac (Clang)
• GitHub Check: Ubuntu (GCC)
• GitHub Check: Ubuntu (Clang)
• GitHub Check: Windows (MSVC)
• GitHub Check: Analyze (c-cpp)

🔇 Additional comments (9)

lib/profiler/src/calc_cpu_freq.c (1)

24-24: LGTM! Floating-point arithmetic improves precision.

The change from integer arithmetic to floating-point arithmetic prevents potential overflow and provides more accurate CPU frequency estimation.

lib/profiler/src/rdtsc.c (1)

2-2: LGTM! Explicit inclusion of stdint.h.

Explicitly including stdint.h for fixed-width integer types improves code clarity and portability.

lib/profiler/src/rep_return_data.c (1)

49-51: LGTM! Essential NULL check added.

The NULL check after malloc is necessary for robustness. Returning NULL on allocation failure allows the caller to handle the error appropriately.

lib/profiler/src/perf.c (3)

11-11: LGTM! Explicit initialization improves clarity.

Explicitly initializing _FD to 0 makes the initial state clear and follows best practices.

---

27-30: LGTM! Prevents resource leaks on repeated setup.

Closing the existing file descriptor before opening a new one prevents resource leaks when perf_setup is called multiple times.

---

63-66: LGTM! Validates read to prevent partial data usage.

Checking that the read returned the expected number of bytes prevents using partially read or invalid data.

lib/profiler/include/macros.h (1)

39-39: LGTM! Macro signature now consistent across branches.

Aligning the TIME_BANDWIDTH_BEGIN macro signature in the non-PROFILER branch with the PROFILER branch ensures consistency and prevents compilation errors when switching between profiling and non-profiling builds.

lib/profiler/include/repetition_tester.h (1)

12-12: LGTM! Follows standard macro idiom.

Removing the trailing semicolon from the macro expansion follows the standard idiom for do-while(0) macros, where the caller supplies the semicolon. This makes macro invocations look like regular statements and avoids issues in certain contexts.

src/main.c (1)

1482-1490: LGTM! Profiler finalization correctly relocated.

Moving profiler finalization to the end of main() after PHYSFS_deinit() ensures all subsystems are properly shut down before writing profiler output. The fallback to stdout if file opening fails is appropriate.