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.

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

cmd/compile: optimize type switch #18492

Open
bradfitz opened this Issue Jan 2, 2017 · 12 comments

Comments

Projects
None yet
8 participants
@bradfitz
Member

bradfitz commented Jan 2, 2017

In code review https://go-review.googlesource.com/c/34773/, John Doe points out that a type switch in an inner loop can be optimized by pulling it out of the loop, setting an integer, and switching on the integer value instead.

John Doe provided this benchmark: https://play.golang.org/p/6LXF82e6U4

With tip, I get:

$ go test -v -bench=. -benchtime=2s
BenchmarkTypeSwitchInside-4         2000           1298710 ns/op
BenchmarkTypeSwitchOutside-4       10000            475899 ns/op

These seem like they could be identical. In both cases, it's just a switch on an integer. In the "Inside" case, that integer just happens to be in the first word of an interface value.

/cc @randall77 @mdempsky @josharian

@bradfitz bradfitz added the Performance label Jan 2, 2017

@bradfitz bradfitz added this to the Go1.9Maybe milestone Jan 2, 2017

@josharian

This comment has been minimized.

Contributor

josharian commented Jan 2, 2017

cc @dr2chase who likes hoisting expensive operations out of loops

@mvdan

This comment has been minimized.

Member

mvdan commented Jan 2, 2017

Just to clarify, is this about optimizing all type switches or just about those that can be moved outside of a loop?

@josharian

This comment has been minimized.

Contributor

josharian commented Jan 3, 2017

As I understand it, moving expensive things out of loops. But maybe I'm missing something.

@minux

This comment has been minimized.

Member

minux commented Jan 3, 2017

@mvdan

This comment has been minimized.

Member

mvdan commented Jan 3, 2017

I agree that the former is more interesting. It's also what I would understand given the issue title.

@randall77

This comment has been minimized.

Contributor

randall77 commented Jan 3, 2017

In go 1.8, the type switch compiles fairly small. The non-empty-interface-to-concrete-type switch compiles to just a few instructions.

func f(t I) int {
	switch t.(type) {
	case *T:
		return 1
	}
	return 0
}

generates for the switch

	0x0000 00000 (tmp1.go:14)	MOVQ	"".t+8(FP), AX    // AX = itab of interface
	0x0005 00005 (tmp1.go:14)	TESTQ	AX, AX
	0x0008 00008 (tmp1.go:14)	JEQ	52                // nil interface
	0x000a 00010 (tmp1.go:14)	MOVQ	8(AX), CX         // CX = type
	0x000e 00014 (tmp1.go:14)	MOVL	16(CX), DX        // DX = hash of type
	0x0011 00017 (tmp1.go:14)	CMPL	DX, $432690315
	0x0017 00023 (tmp1.go:14)	JNE	52
	0x0019 00025 (tmp1.go:14)	TESTQ	AX, AX
	0x001c 00028 (tmp1.go:14)	JEQ	$0, 62
	0x001e 00030 (tmp1.go:18)	LEAQ	type.*"".T(SB), AX
	0x0025 00037 (tmp1.go:14)	CMPQ	AX, CX
	0x0028 00040 (tmp1.go:14)	JNE	$0, 52
	0x002a 00042 (tmp1.go:16)	MOVQ	$1, "".~r1+24(FP)
	0x0033 00051 (tmp1.go:16)	RET
	0x0034 00052 (tmp1.go:18)	MOVQ	$0, "".~r1+24(FP)
	0x003d 00061 (tmp1.go:18)	RET
	0x003e 00062 (tmp1.go:14)	MOVQ	AX, CX
	0x0041 00065 (tmp1.go:18)	JMP	30

It's not optimal yet (the redundant TESTQ), but it's pretty small. The code is even slightly smaller when switching on empty interfaces.

Possibly for small switches we could get rid of the hash comparison and check the type pointers directly. (The hash enables binary search. We'd have to do linear search if we just used the type pointers.)

For nonempty interfaces, maybe we could even compare the itab with the address of the now-statically-known I/*T itab struct? Then we wouldn't even need to load the type out of the itab.

The more general question, how to pull this stuff out of the loop altogether, is harder. It involves memory operations so it isn't clear lifting is legal just from looking at the SSA form. We'd have to understand that some of the loads are from never-changing memory locations.

@gopherbot

This comment has been minimized.

gopherbot commented Jan 4, 2017

CL https://golang.org/cl/34810 mentions this issue.

@rasky

This comment has been minimized.

Member

rasky commented Jan 4, 2017

For larger switches, given that we already have a compile-time known hash per type, it looks like it would be quite easy to use something along the lines of MRST. In other words, a small sequence of bits in those hashes could be used as perfect hashing to index a small jump table.

@randall77

This comment has been minimized.

Contributor

randall77 commented Jan 4, 2017

Jump tables: #5496, #10870

@josharian

This comment has been minimized.

Contributor

josharian commented Jan 4, 2017

Jump tables see also #15780 (comment)

gopherbot pushed a commit that referenced this issue Feb 13, 2017

cmd/compile: optimize non-empty-interface type conversions
When doing i.(T) for non-empty-interface i and concrete type T,
there's no need to read the type out of the itab. Just compare the
itab to the itab we expect for that interface/type pair.

Also optimize type switches by putting the type hash of the
concrete type in the itab. That way we don't need to load the
type pointer out of the itab.

Update #18492

Change-Id: I49e280a21e5687e771db5b8a56b685291ac168ce
Reviewed-on: https://go-review.googlesource.com/34810
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
@opennota

This comment has been minimized.

opennota commented Jun 30, 2017

With go1.9beta2, I get

BenchmarkTypeSwitchInside-4         2000            687881 ns/op
BenchmarkTypeSwitchOutside-4        3000            443001 ns/op

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9Maybe Jun 30, 2017

@bradfitz bradfitz added the NeedsFix label Jun 30, 2017

@bradfitz bradfitz modified the milestones: Go1.10, Go1.11 Nov 28, 2017

@bradfitz bradfitz modified the milestones: Go1.11, Unplanned May 18, 2018

@opennota

This comment has been minimized.

opennota commented Nov 19, 2018

Go 1.11.2:

BenchmarkTypeSwitchInside-4         3000            513823 ns/op
BenchmarkTypeSwitchOutside-4        3000            444997 ns/op
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment