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

os: make File.WriteString not allocate #42406

Open
josharian opened this issue Nov 5, 2020 · 5 comments
Open

os: make File.WriteString not allocate #42406

josharian opened this issue Nov 5, 2020 · 5 comments

Comments

@josharian
Copy link
Contributor

@josharian josharian commented Nov 5, 2020

This is os.File.WriteString's implementation:

// WriteString is like Write, but writes the contents of string s rather than
// a slice of bytes.
func (f *File) WriteString(s string) (n int, err error) {
	return f.Write([]byte(s))
}

This currently accounts for 10% 35% of allocated space in a service I'm maintaining.

We should be able to do an unsafe cast of s to b here, and avoid the allocation.

Do any kernels modify the bytes during Write? Any other reason why we wouldn't be able to do this?

cc @bradfitz

@josharian josharian added this to the Go1.17 milestone Nov 5, 2020
@josharian
Copy link
Contributor Author

@josharian josharian commented Nov 6, 2020

Problem: os can't import reflect, at least according to existing dependency rules. And I don't think #19367 will help here.

There are options: add go:linkname with package runtime support, allow os to import reflect, add StringHeader/SliceHeader to package reflectlite and teach the compiler about that. I imagine the first is the most palatable.

@odeke-em
Copy link
Member

@odeke-em odeke-em commented Nov 6, 2020

@josharian you are Josh and can make it work perhaps in the compiler by special recognition, if the symbol is a (*os.File).WriteString and its on an architecture that we've audited as won't be mutating bytes, rewrite it :)

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 6, 2020

@josharian Permit me to introduce you to the exciting new internal/unsafeheader package.

@josharian
Copy link
Contributor Author

@josharian josharian commented Nov 6, 2020

Oooooooooooooooooh!

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 7, 2020

Change https://golang.org/cl/268020 mentions this issue: os: avoid allocation in File.WriteString

josharian added a commit to tailscale/go that referenced this issue Nov 19, 2020
Instead of alloc+copy to convert the string
to a byte slice, do an unsafe conversion.

Rely on the kernel not to scribble on the
buffer during the write.

Fixes golang#42406

Change-Id: I66f4838b43a11bcc3d67bbfa1706726318d55343
josharian added a commit to tailscale/go that referenced this issue Nov 19, 2020
[out for review upstream: https://go-review.googlesource.com/c/go/+/268020]

Instead of alloc+copy to convert the string
to a byte slice, do an unsafe conversion.

Rely on the kernel not to scribble on the
buffer during the write.

Fixes golang#42406

Change-Id: I66f4838b43a11bcc3d67bbfa1706726318d55343
josharian added a commit to tailscale/go that referenced this issue Nov 19, 2020
[out for review upstream: https://go-review.googlesource.com/c/go/+/268020]

Instead of alloc+copy to convert the string
to a byte slice, do an unsafe conversion.

Rely on the kernel not to scribble on the
buffer during the write.

Fixes golang#42406

Change-Id: I66f4838b43a11bcc3d67bbfa1706726318d55343
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.