refactor: use b.Loop() to simplify the code

[zhoujiaweii]

Overview

replace for i := range b.N or for range b.N in a benchmark with for b.Loop(), and remove any preceding calls to b.StopTimer, b.StartTimer, and b.ResetTimer.

Supported by Go Team, more info: https://go.dev/blog/testing-b-loop

[gemini-code-assist]

Summary of Changes

Hello @zhoujiaweii, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the existing Go benchmark implementations across several test files to adopt the b.Loop() construct. This change modernizes the benchmark code, making it more concise and leveraging the Go testing package's built-in optimizations for managing benchmark iterations, ultimately leading to more accurate and robust performance measurements by allowing the Go runtime to dynamically adjust the number of iterations.

Highlights

• Benchmark Loop Refactor: All benchmark loops, previously written as for n := 0; n < b.N; n++, have been updated to use the more idiomatic and efficient for b.Loop() construct.
• Go Best Practices: This change aligns with Go's recommended benchmarking practices, allowing the Go runtime to manage benchmark iterations more effectively and dynamically.
• Code Simplification: The refactoring simplifies the benchmark code by abstracting the loop management, making the benchmarks cleaner and easier to read.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request aims to modernize benchmark loops using the new b.Loop() helper from Go 1.22. While the intention is good, there seems to be a misunderstanding of how b.Loop() works, leading to broken benchmarks in most of the changes.

The b.Loop() helper is designed for benchmarks that have per-iteration setup code that should not be timed. It works by calling b.StopTimer() at the beginning of each loop iteration. This means that to time any code within the loop, you must explicitly call b.StartTimer().

For simple benchmarks where the entire loop body should be timed, the traditional for i := 0; i < b.N; i++ loop is still the correct and idiomatic approach. Changing them to for b.Loop() { ... } without adding b.StartTimer() causes the benchmark to measure nothing, effectively breaking it.

Most of the changes in this PR fall into this category and should be reverted. However, BenchmarkRepair in extendeddatacrossword_test.go is a good candidate for b.Loop(), but the implementation is slightly off. My specific comments will provide details.

[rootulp]

For simple benchmarks where the entire loop body should be timed, the traditional for i := 0; i < b.N; i++ loop is still the correct and idiomatic approach. Changing them to for b.Loop() { ... } without adding b.StartTimer() causes the benchmark to measure nothing, effectively breaking it.

Gemini code assist seems incorrect based on the article: https://go.dev/blog/testing-b-loop#when-to-use

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.03%. Comparing base (d6c118c) to head (e736b47).
⚠️ Report is 53 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #380      +/-   ##
==========================================
+ Coverage   80.89%   83.03%   +2.13%     
==========================================
  Files           8        7       -1     
  Lines         869      831      -38     
==========================================
- Hits          703      690      -13     
+ Misses        119      102      -17     
+ Partials       47       39       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rootulp]

Thanks for the link in the PR description. I tested manually and running benchmarks still works.

$ make bench
--> Running benchmarks
goos: darwin
goarch: amd64
pkg: github.com/celestiaorg/rsmt2d
cpu: VirtualApple @ 2.50GHz
BenchmarkEncoding/Leopard_128_shares_512-12         	   33666	     36232 ns/op	   99139 B/op	     132 allocs/op
BenchmarkDecoding/Leopard_128_shares_512-12         	 5788555	       204.8 ns/op	       1 B/op	       0 allocs/op
BenchmarkEDSRootsWithDefaultTree/32x32x512_ODS-12   	     457	   2357882 ns/op	 1810815 B/op	   34055 allocs/op
BenchmarkEDSRootsWithDefaultTree/64x64x512_ODS-12   	     170	   7042752 ns/op	 6241166 B/op	  133638 allocs/op
BenchmarkEDSRootsWithDefaultTree/128x128x512_ODS-12 	      46	  26405020 ns/op	26508937 B/op	  530444 allocs/op
BenchmarkEDSRootsWithDefaultTree/256x256x512_ODS-12 	      10	 102919612 ns/op	108873646 B/op	 2110517 allocs/op
BenchmarkEDSRootsWithDefaultTree/512x512x512_ODS-12 	       3	 429154819 ns/op	497299274 B/op	 8419658 allocs/op
BenchmarkEDSRootsWithErasuredNMT/32x32x512_ODS=0MB,_EDS=0MB-12         	     304	   3894178 ns/op	 9934634 B/op	   68615 allocs/op
BenchmarkEDSRootsWithErasuredNMT/64x64x512_ODS=2MB,_EDS=8MB-12         	      96	  12798105 ns/op	39406556 B/op	  268813 allocs/op
BenchmarkEDSRootsWithErasuredNMT/128x128x512_ODS=8MB,_EDS=32MB-12      	      28	  43403719 ns/op	157211487 B/op	 1062941 allocs/op
BenchmarkEDSRootsWithErasuredNMT/256x256x512_ODS=32MB,_EDS=128MB-12    	       7	 162644226 ns/op	632962194 B/op	 4225080 allocs/op
BenchmarkEDSRootsWithErasuredNMT/512x512x512_ODS=128MB,_EDS=512MB-12   	       2	 668446312 ns/op	2538550608 B/op	16849123 allocs/op
BenchmarkEDSRootsWithErasuredNMT/1024x1024x512_ODS=512MB,_EDS=2048MB-12         	       1	3321605667 ns/op	10123395664 B/op	67291195 allocs/op
BenchmarkRepair/Leopard_4x4x512_ODS-12                                          	    4162	    282716 ns/op	  175282 B/op	     795 allocs/op
BenchmarkRepair/Leopard_8x8x512_ODS-12                                          	    1086	   1076318 ns/op	  587170 B/op	    2902 allocs/op
BenchmarkRepair/Leopard_16x16x512_ODS-12                                        	     261	   4610408 ns/op	 2219682 B/op	   10939 allocs/op
BenchmarkRepair/Leopard_32x32x512_ODS-12                                        	      67	  18378841 ns/op	 8763665 B/op	   42347 allocs/op
BenchmarkRepair/Leopard_64x64x512_ODS-12                                        	      16	  71317930 ns/op	31547549 B/op	  166267 allocs/op
BenchmarkRepair/Leopard_128x128x512_ODS-12                                      	       4	 288566198 ns/op	114510030 B/op	  660987 allocs/op
BenchmarkRepair/Leopard_256x256x512_ODS-12                                      	       1	1571007917 ns/op	441955728 B/op	 2638328 allocs/op
BenchmarkRepair/Leopard_512x512x512_ODS-12                                      	       1	6325131916 ns/op	1727389704 B/op	10521550 allocs/op
BenchmarkExtensionEncoding/Leopard_4x4x512_ODS-12                               	   47174	     25347 ns/op	   38737 B/op	     153 allocs/op
BenchmarkExtensionEncoding/Leopard_8x8x512_ODS-12                               	   21165	     56568 ns/op	  150677 B/op	     389 allocs/op
BenchmarkExtensionEncoding/Leopard_16x16x512_ODS-12                             	    9349	    137029 ns/op	  629468 B/op	    1149 allocs/op
BenchmarkExtensionEncoding/Leopard_32x32x512_ODS-12                             	    3782	    315953 ns/op	 2644674 B/op	    3821 allocs/op
BenchmarkExtensionEncoding/Leopard_64x64x512_ODS-12                             	    1206	    966341 ns/op	10576486 B/op	   13775 allocs/op
BenchmarkExtensionEncoding/Leopard_128x128x512_ODS-12                           	     320	   3723408 ns/op	43625259 B/op	   52114 allocs/op
BenchmarkExtensionEncoding/Leopard_256x256x512_ODS-12                           	     100	  17432046 ns/op	127566591 B/op	  200990 allocs/op
BenchmarkExtensionEncoding/Leopard_512x512x512_ODS-12                           	      16	  68825951 ns/op	514127503 B/op	  795177 allocs/op
BenchmarkExtensionWithRoots/Leopard_4x4x512_ODS-12                              	   10000	    110292 ns/op	  120244 B/op	     830 allocs/op
BenchmarkExtensionWithRoots/Leopard_8x8x512_ODS-12                              	    5217	    228580 ns/op	  360338 B/op	    2794 allocs/op
BenchmarkExtensionWithRoots/Leopard_16x16x512_ODS-12                            	    1920	    620393 ns/op	 1212964 B/op	   10050 allocs/op
BenchmarkExtensionWithRoots/Leopard_32x32x512_ODS-12                            	     596	   2082090 ns/op	 4484298 B/op	   38002 allocs/op
BenchmarkExtensionWithRoots/Leopard_64x64x512_ODS-12                            	     160	   7367285 ns/op	17436658 B/op	  147668 allocs/op
BenchmarkExtensionWithRoots/Leopard_128x128x512_ODS-12                          	      38	  28928776 ns/op	73217024 B/op	  583065 allocs/op
BenchmarkExtensionWithRoots/Leopard_256x256x512_ODS-12                          	       9	 120489625 ns/op	236774602 B/op	 2312489 allocs/op
BenchmarkExtensionWithRoots/Leopard_512x512x512_ODS-12                          	       2	 515074730 ns/op	1014404156 B/op	 9216588 allocs/op
PASS
ok  	github.com/celestiaorg/rsmt2d	61.647s