Skip to content

predicates_test: Fix BenchmarkSign measuring empty loop#269

Open
xl4624 wants to merge 1 commit into
golang:masterfrom
xl4624:predicate-sign-no-inline
Open

predicates_test: Fix BenchmarkSign measuring empty loop#269
xl4624 wants to merge 1 commit into
golang:masterfrom
xl4624:predicate-sign-no-inline

Conversation

@xl4624
Copy link
Copy Markdown
Contributor

@xl4624 xl4624 commented Apr 30, 2026

Comparing assemblies from objdump:

> diff -u /tmp/before.s /tmp/after.s
--- /tmp/before.s	2026-04-30 00:43:25.516557064 -0700
+++ /tmp/after.s	2026-04-30 00:43:35.893749762 -0700
@@ -1,7 +1,33 @@
+  493b6610 CMPQ SP, 0x10(R14)
+  7671 JBE 0x70ff97
+  55 PUSHQ BP
+  4889e5 MOVQ SP, BP
+  4883ec50 SUBQ $0x50, SP
+  4889442460 MOVQ AX, 0x60(SP)
   31c9 XORL CX, CX
-  eb03 JMP 0x70fec7
+  eb51 JMP 0x70ff88
+  48894c2448 MOVQ CX, 0x48(SP)
+  f20f10051cad0f00 MOVSD_XMM $f64.c008000000000000(SB), X0
+  f20f100d6ca00f00 MOVSD_XMM 0xfa06c(IP), X1
+  f20f10155ca00f00 MOVSD_XMM 0xfa05c(IP), X2
+  f20f101dd4a00f00 MOVSD_XMM 0xfa0d4(IP), X3
+  0f10e1 MOVUPS X1, X4
+  0f10e8 MOVUPS X0, X5
+  f20f10355ea00f00 MOVSD_XMM 0xfa05e(IP), X6
+  f20f103dceac0f00 MOVSD_XMM $f64.c000000000000000(SB), X7
+  450f57c0 XORPS X8, X8
+  e845ffffff CALL github.com/golang/geo/s2.signNoInline(SB)
+  488b4c2448 MOVQ 0x48(SP), CX
   48ffc1 INCQ CX
+  488b442460 MOVQ 0x60(SP), AX
   48398810020000 CMPQ 0x210(AX), CX
-  7ff4 JG 0x70fec4
+  7fa6 JG 0x70ff37
+  4883c450 ADDQ $0x50, SP
+  5d POPQ BP
   c3 RET
+  4889442408 MOVQ AX, 0x8(SP)
+  0f1f4000 NOPL 0(AX)
+  e81b14d8ff CALL runtime.morestack_noctxt.abi0(SB)
+  488b442408 MOVQ 0x8(SP), AX
+  e971ffffff JMP github.com/golang/geo/s2.BenchmarkSign(SB)

Benchmarks:

> benchstat /tmp/before.txt /tmp/after.txt
goos: linux
goarch: amd64
pkg: github.com/golang/geo/s2
cpu: AMD Ryzen 7 PRO 8840HS w/ Radeon 780M Graphics
        │ /tmp/before.txt │             /tmp/after.txt             │
        │     sec/op      │    sec/op      vs base                 │
Sign-16      0.2083n ± 2%   1.5070n ± 13%  +623.48% (p=0.000 n=10)

@jmr
Copy link
Copy Markdown
Collaborator

jmr commented Apr 30, 2026

These don't look like great benchmarks either way. They measure throughput, not latency. It would be better to create an array of points, then call Sign(pts[j], pts[j + 1], pts[j + 2]) and update j based on the result.

Is there a reason you're interested in them? Are you going to optimize Sign? Or you just noticed an improbably fast timing?

@xl4624
Copy link
Copy Markdown
Contributor Author

xl4624 commented May 1, 2026

Is there a reason you're interested in them? Are you going to optimize Sign? Or you just noticed an improbably fast timing?

Yeah, just noticed the numbers were off, no plans to optimize anything (I don't really use Go). Happy to redo these with an array if you'd like.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@xl4624 @jmr