Skip to content
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

Jump to bottom

cmd/compile: avoid updating capacity if unused #23832

Open
dsnet opened this issue Feb 14, 2018 · 1 comment
Open

cmd/compile: avoid updating capacity if unused #23832

dsnet opened this issue Feb 14, 2018 · 1 comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone
Unplanned

Comments

@dsnet
Copy link
Member

dsnet commented Feb 14, 2018 •
edited

Using go1.10,

Consider the following code:

func IsASCII(b []byte) bool {
	for len(b) >= 8 {
		x := binary.LittleEndian.Uint64(b)
		if x&0x8080808080808080 > 0 {
			return false
		}
		b = b[8:]
	}
	for _, x := range b {
		if x&0x80 > 0 {
			return false
		}
	}
	return true
}

The compiled assembly is as follows:

0x0000 00000 (main.go:11)	MOVQ	"".b+16(SP), AX
0x0005 00005 (main.go:11)	MOVQ	"".b+24(SP), CX
0x000a 00010 (main.go:11)	MOVQ	"".b+8(SP), DX
0x000f 00015 (main.go:12)	JMP	45
0x0011 00017 (main.go:17)	ADDQ	$-8, CX
0x0015 00021 (main.go:17)	MOVQ	CX, BX
0x0018 00024 (main.go:17)	NEGQ	CX
0x001b 00027 (main.go:17)	SARQ	$63, CX
0x001f 00031 (main.go:17)	ANDQ	$8, CX
0x0023 00035 (main.go:17)	ADDQ	CX, DX
0x0026 00038 (main.go:17)	ADDQ	$-8, AX
0x002a 00042 (main.go:17)	MOVQ	BX, CX
0x002d 00045 (main.go:12)	CMPQ	AX, $8
0x0031 00049 (main.go:12)	JLT	75
0x0033 00051 (main.go:13)	MOVQ	(DX), BX
0x0036 00054 (main.go:13)	MOVQ	$-9187201950435737472, SI
0x0040 00064 (main.go:14)	TESTQ	BX, SI
0x0043 00067 (main.go:14)	JLS	17
0x0045 00069 (main.go:15)	MOVB	$0, "".~r1+32(SP)
0x004a 00074 (main.go:15)	RET
0x004b 00075 (main.go:15)	MOVL	$0, CX
0x004d 00077 (main.go:19)	JMP	85
0x004f 00079 (main.go:19)	INCQ	DX
0x0052 00082 (main.go:19)	INCQ	CX
0x0055 00085 (main.go:19)	CMPQ	CX, AX
0x0058 00088 (main.go:19)	JGE	104
0x005a 00090 (main.go:19)	MOVBLZX	(DX), BX
0x005d 00093 (main.go:20)	TESTB	$-128, BL
0x0060 00096 (main.go:20)	JLS	79
0x0062 00098 (main.go:21)	MOVB	$0, "".~r1+32(SP)
0x0067 00103 (main.go:21)	RET
0x0068 00104 (main.go:24)	MOVB	$1, "".~r1+32(SP)
0x006d 00109 (main.go:24)	RET

You'll notice that the slice header {ptr, len, cap} are loaded into {DX, AX, CX}. Several times in the assembly, you see the CX register being used as it seems that b = b[8:] on line 17 causes the capacity to also be updated.

However, the go code actually does not care about the capacity at all. If the compiler is able to prove that the capacity will never be used, then it could avoid doing the math to ensure that the capacity is up-to-date.

\cc @randall77 @dr2chase
@randall77
Copy link
Contributor

Looks like the capacity is just used to avoid the points-past-the-end-of-the-object issue.
See issue #9401 and friends.
To sum that issue up, when we do b = b[i:] we can't just do b.ptr+=i; b.len-=i; b.cap-=i because that might push b.ptr up enough to point to the next object in memory.
So instead we intentionally avoid incrementing b.ptr if i == b.cap (but with shifts and masks to make it branchless). That's what cap is used for in this loop.

It's possible because there are no safepoints, that we could avoid the use of cap here. I believe that's just issue #15397. But @aclements is thinking about safepoints everywhere, which might throw a wrench in ever fixing that issue...

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 28, 2018
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 28, 2018
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@ianlancetaylor @dsnet @randall77 @gopherbot