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

proposal: the make function needs to be optimized for large slices #23906

Closed
go101 opened this issue Feb 18, 2018 · 8 comments
Closed

proposal: the make function needs to be optimized for large slices #23906

go101 opened this issue Feb 18, 2018 · 8 comments
Labels
FrozenDueToAge Proposal
Milestone
Proposal

Comments

@go101
Copy link

go101 commented Feb 18, 2018 •
edited

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.10 linux/amd64

Does this issue reproduce with the latest release?

yes

What did you do?

package main

import (
	"testing"
)

const N = 10000
type Element int64
var x = make([]Element, N)
var yForCopy = make([]Element, N)
var yForMake []Element
var yForMakeCopy []Element
var yForAppend []Element

func Benchmark_PureCopy(b *testing.B) {
	for i := 0; i < b.N; i++ {
		copy(yForCopy, x)
	}
}

func Benchmark_PureMake(b *testing.B) {
	for i := 0; i < b.N; i++ {
		yForMake = make([]Element, N)
	}
}

func Benchmark_MakeAndCopy(b *testing.B) {
	for i := 0; i < b.N; i++ {
		yForMakeCopy = make([]Element, N)
		copy(yForMakeCopy, x)
	}
}

func Benchmark_Append(b *testing.B) {
	for i := 0; i < b.N; i++ {
		yForAppend = append([]Element(nil), x...)
	}
}

What did you expect to see?

The append call should spend more time than the pure make call and spend almost same time as the make+copy calls.

However, these are only true if N is small (<= 1000).

What did you see instead?

When N == 10

goos: linux
goarch: amd64
Benchmark_PureCopy-4      	50000000	        30.7 ns/op
Benchmark_PureMake-4      	 5000000	       387 ns/op
Benchmark_MakeAndCopy-4   	 3000000	       446 ns/op
Benchmark_Append-4        	 3000000	       489 ns/op

When N == 1000

goos: linux
goarch: amd64
Benchmark_PureCopy-4      	 5000000	       259 ns/op
Benchmark_PureMake-4      	  500000	      2892 ns/op
Benchmark_MakeAndCopy-4   	  500000	      3582 ns/op
Benchmark_Append-4        	  500000	      3287 ns/op

When N == 10000

goos: linux
goarch: amd64
Benchmark_PureCopy-4      	  500000	      3537 ns/op
Benchmark_PureMake-4      	  100000	     21456 ns/op
Benchmark_MakeAndCopy-4   	   50000	     29679 ns/op
Benchmark_Append-4        	  100000	     17697 ns/op
@gopherbot gopherbot added this to the Proposal milestone Feb 18, 2018
@odeke-em
Copy link
Member

/cc @josharian

@sywesk
Copy link

sywesk commented Feb 18, 2018

Isn't that due to this PR ? 2b41554

It states that:

This change also makes it possible to allocate individual objects
larger than 512GB. As a result, a few tests that expected huge
allocations to fail needed to be changed to make even larger
allocations. However, at the moment attempting to allocate a humongous
object may cause the program to freeze for several minutes on Linux as
we fall back to probing every page with addrspace_free. That logic
(and this failure mode) will be removed in the next CL.

@go101
Copy link
Author

go101 commented Feb 18, 2018

This should be unrelated. The issue is also for Go 1.9.

@josharian
Copy link
Contributor

Duplicate of #21266?

@martisch
Copy link
Contributor

I think we can break this into multiple performance issues instead of a proposal:

  1. memclr (make) can be faster or slower than memmove (append) depending on size see runtime: memmove sometimes faster than memclrNoHeapPointers #23306.
  2. the compiler could be made to optimize make + copy of same length to use uninitialized allocation where possible (like append) instead of first clearing the memory then overwriting it. (to be discussed)

@go101
Copy link
Author

go101 commented Feb 19, 2018

@josharian yes, it is a duplicate of #21266
But this is one has more details.

@bradfitz
Copy link
Contributor

Closing as duplicate of #21266. I'll drop a link to this bug from that one. Or you can comment there.

Optimizations don't need to be proposals, either.

@bradfitz bradfitz mentioned this issue Feb 19, 2018
Closed
@go101
Copy link
Author

go101 commented Feb 20, 2018

I'm sorry, I mean this is a duplicated of #23306

@golang golang locked and limited conversation to collaborators Feb 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
7 participants
@bradfitz @josharian @martisch @sywesk @odeke-em @gopherbot @go101