Speedup C# version by using double type for rounds variable #248

karelg · 2025-12-15T15:46:37Z

This implementation was consistently faster in my testing.

Summary by CodeRabbit

Refactor
- Updated round value parsing to support decimal values instead of whole numbers, with corresponding adjustments to iteration boundaries.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-15T15:46:51Z

Walkthrough

The rounds variable in src/leibniz.cs is changed from int to double, with corresponding updates to the file parsing logic and loop boundary condition to accommodate the new floating-point type.

Changes

Cohort / File(s)	Summary
Type conversion `src/leibniz.cs`	Changed rounds parsing from int to double; updated rounds variable declaration and loop condition to use double-based iteration boundary.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Straightforward type conversion with consistent application across a single file
No complex logic changes or multi-component interactions to verify
Primary focus should be confirming the loop boundary behavior with the double type remains intended

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title claims a performance improvement from using double instead of int, but the summary shows only a type change and altered loop condition (i < rounds + 2), with no clear connection to speed gains.	Revise the title to accurately reflect the change: either describe the specific type change without claiming a speedup benefit, or provide technical justification for why double improves performance in this context.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/leibniz.cs (2)
1-1: Consider keeping rounds as an integer iteration count

rounds is conceptually “number of iterations”, so using double here is non‑idiomatic and also permits non‑integral values from rounds.txt. Given it’s only used as a loop bound, an int keeps intent clearer and avoids surprising inputs.

If your benchmarks still show a win with double, it’d be good to document that in a comment; otherwise I’d keep this as an int:
-var rounds = double.Parse(File.ReadAllText("rounds.txt"));
+var rounds = int.Parse(File.ReadAllText("rounds.txt"));
As per coding guidelines, aim for idiomatic code in src/leibniz.*.

6-9: Avoid mixing int loop index with a double upper bound

With var i = 2; inferred as int and rounds as double, the condition i < rounds + 2 forces an implicit numeric conversion on every iteration. That may offset or reverse any performance gain you’re aiming for and hurts readability a bit.

Two cleaner options:

Keep everything integral (most idiomatic for counts):
-var rounds = double.Parse(File.ReadAllText("rounds.txt"));
+var rounds = int.Parse(File.ReadAllText("rounds.txt"));

-for (var i = 2; i < rounds + 2; i++) {
+for (var i = 2; i < rounds + 2; i++) {
    x = -x;
    pi += x / (2 * i - 1);
}
Or, if you really want a double-based loop, make the index double too:
var rounds = double.Parse(File.ReadAllText("rounds.txt"));

for (double i = 2.0; i < rounds + 2.0; i += 1.0) {
    x = -x;
    pi += x / (2.0 * i - 1.0);
}
I’d benchmark both variants; integer bounds are usually friendlier to the JIT for tight loops.

As per coding guidelines, favor clear, idiomatic constructs in src/leibniz.*.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 83dc041 and be3e253.

📒 Files selected for processing (1)

src/leibniz.cs (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

src/leibniz.*

📄 CodeRabbit inference engine (CLAUDE.md)

src/leibniz.*: Create source file following the naming convention src/leibniz.<ext> when adding a new language implementation
Implementations must be single-threaded with no concurrency, parallelism, multi-threading, async, or parallel processing
All implementations must use the Leibniz formula for calculating π
Use idiomatic code with standard language features; compiler optimization flags are allowed, and auto-vectorization hints like @simd in Julia are acceptable

Files:

src/leibniz.cs

⚙️ CodeRabbit configuration file

src/leibniz.*: This is a benchmark implementation using the Leibniz formula to calculate π.
Review for:

Single-threaded execution only (no concurrency/parallelism)

Correct Leibniz formula implementation

Potential performance optimizations that don't involve parallelism

SIMD is allowed but should be in separate files (e.g., leibniz_simd.*)

Files:

src/leibniz.cs

🧠 Learnings (2)

📓 Common learnings

Learnt from: CR
Repo: niklas-heer/speed-comparison PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T06:40:02.627Z
Learning: Applies to src/leibniz.* : Use idiomatic code with standard language features; compiler optimization flags are allowed, and auto-vectorization hints like `simd` in Julia are acceptable

Learnt from: CR
Repo: niklas-heer/speed-comparison PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T06:40:02.627Z
Learning: Applies to src/leibniz.{c,cpp,cc,cxx,h,hpp} : In C/C++ implementations, declare variables inside loops to enable auto-vectorization

Learnt from: CR
Repo: niklas-heer/speed-comparison PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T06:40:02.627Z
Learning: Applies to src/leibniz.* : All implementations must use the Leibniz formula for calculating π

📚 Learning: 2025-12-14T06:40:02.627Z

Learnt from: CR
Repo: niklas-heer/speed-comparison PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T06:40:02.627Z
Learning: Applies to src/leibniz.* : Use idiomatic code with standard language features; compiler optimization flags are allowed, and auto-vectorization hints like `simd` in Julia are acceptable

Applied to files:

src/leibniz.cs

🧬 Code graph analysis (1)

src/leibniz.cs (1)

src/leibniz.js (1)

rounds (4-4)

neon-sunset · 2025-12-15T18:00:46Z

FYI this tanks performance on ARM64. It's worth a try but I think having the submissions optimize for the runner hardware is flawed. I guess other languages may/will do it also, but it's unfortunate since it makes results less interesting because adding very specific changes may confuse casual readers which in practice won't be able to apply the same insight.

Speedup C# version by using double type for rounds

be3e253

coderabbitai bot reviewed Dec 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

U 8000 h oh!

Speedup C# version by using double type for rounds variable #248

Speedup C# version by using double type for rounds variable #248

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Speedup C# version by using double type for rounds variable #248

Are you sure you want to change the base?

Speedup C# version by using double type for rounds variable #248

Conversation

Uh oh!

Summary by CodeRabbit

Uh oh!

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants