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: (*File).Write should not allocate #9370

Closed
jeffallen opened this issue Dec 17, 2014 · 5 comments
Closed

os: (*File).Write should not allocate #9370

jeffallen opened this issue Dec 17, 2014 · 5 comments

Comments

@jeffallen
Copy link
Contributor

@jeffallen jeffallen commented Dec 17, 2014

On tip (go version devel +6262902) the following test program shows that os.(*File).Write makes 2 allocations. http://play.golang.org/p/6tjQO4vdlE

(You can't see the problem on playground but you can see it when running locally.)

It would be better if os.(_File).Write did not allocate, as it is a hot path in typical Go servers. For comparison, os.(_File).Read does not allocate.

See also: https://groups.google.com/forum/#!searchin/golang-nuts/Write$20allocates/golang-nuts/0hfeLJP1LSk/u5QfV6cDIsoJ

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Dec 17, 2014

This is issue #8618.

@jeffallen

This comment has been minimized.

Copy link
Contributor Author

@jeffallen jeffallen commented Dec 17, 2014

Well, last time we talked (in that thread) you agreed that it might be worth patching since os is right at the core of the std library.

This is the fix: https://go-review.googlesource.com/1730

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Dec 22, 2014

This feels different than #8618. This is about the compiler optimizing concrete type == interface value to not create an interface value when the types differ.

@bradfitz bradfitz reopened this Dec 22, 2014
@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Dec 22, 2014

/cc @rsc, who wanted this open from https://go-review.googlesource.com/#/c/1730/ "Let's just fix the compiler instead. It can do those comparisons without creating interface values."

@mikioh mikioh changed the title os.(*File).Write should not allocate os: (*File).Write should not allocate Dec 23, 2014
@josharian

This comment has been minimized.

Copy link
Contributor

@josharian josharian commented Dec 24, 2014

Started on this. WIP (still rough) at https://github.com/josharian/go/compare/9370. Input welcomed.

josharian added a commit to josharian/go that referenced this issue Dec 25, 2014
Consider an interface value i of type I and concrete value c of type C.

Prior to this CL, i==c was evaluated as
	I(c) == i

As of Go 1.4, all interfaces hold pointers, so evaluating I(c) allocates.

This CL changes the evaluation of i==c to
	x, ok := i.(C); ok && x == c

The new generated code is shorter, uses less stack space,
does not allocate, and makes one runtime call instead of two.

This kind of comparison occurs in 38 places in the stdlib,
mostly in the net and os packages.

benchmark                     old ns/op     new ns/op     delta
BenchmarkCmpIfaceConcrete     31.6          7.92          -74.94%
BenchmarkCmpEfaceConcrete     29.7          7.91          -73.37%

Fixes golang#9370.

Change-Id: I7c4555950bcd6406ee5c613be1f2128da2c9a2b7
josharian added a commit to josharian/go that referenced this issue Dec 25, 2014
Consider an interface value i of type I and concrete value c of type C.

Prior to this CL, i==c was evaluated as
	I(c) == i

As of Go 1.4, all interfaces hold pointers, so evaluating I(c) allocates.

This CL changes the evaluation of i==c to
	x, ok := i.(C); ok && x == c

The new generated code is shorter, uses less stack space,
does not allocate, and makes one runtime call instead of two.

This kind of comparison occurs in 38 places in the stdlib,
mostly in the net and os packages.

benchmark                     old ns/op     new ns/op     delta
BenchmarkCmpIfaceConcrete     31.6          7.92          -74.94%
BenchmarkCmpEfaceConcrete     29.7          7.91          -73.37%

Fixes golang#9370.

Change-Id: I7c4555950bcd6406ee5c613be1f2128da2c9a2b7
josharian added a commit to josharian/go that referenced this issue Dec 25, 2014
Consider an interface value i of type I and concrete value c of type C.

Prior to this CL, i==c was evaluated as
	I(c) == i

As of Go 1.4, all interfaces hold pointers, so evaluating I(c) allocates.

This CL changes the evaluation of i==c to
	x, ok := i.(C); ok && x == c

The new generated code is shorter, uses less stack space,
does not allocate, and makes one runtime call instead of two.

This kind of comparison occurs in 38 places in the stdlib,
mostly in the net and os packages.

benchmark                     old ns/op     new ns/op     delta
BenchmarkCmpIfaceConcrete     31.6          7.92          -74.94%
BenchmarkCmpEfaceConcrete     29.7          7.91          -73.37%

Fixes golang#9370.

Change-Id: I7c4555950bcd6406ee5c613be1f2128da2c9a2b7
josharian added a commit to josharian/go that referenced this issue Dec 25, 2014
Consider an interface value i of type I and concrete value c of type C.

Prior to this CL, i==c was evaluated as
	I(c) == i

As of Go 1.4, all interfaces hold pointers, so evaluating I(c) allocates.

This CL changes the evaluation of i==c to
	x, ok := i.(C); ok && x == c

The new generated code is shorter, uses less stack space,
does not allocate, and makes one runtime call instead of two.

This kind of comparison occurs in 38 places in the stdlib,
mostly in the net and os packages.

benchmark                     old ns/op     new ns/op     delta
BenchmarkCmpIfaceConcrete     31.6          7.92          -74.94%
BenchmarkCmpEfaceConcrete     29.7          7.91          -73.37%

Fixes golang#9370.

Change-Id: I7c4555950bcd6406ee5c613be1f2128da2c9a2b7
josharian added a commit to josharian/go that referenced this issue Dec 25, 2014
Consider an interface value i of type I and concrete value c of type C.

Prior to this CL, i==c was evaluated as
	I(c) == i

Evaluating I(c) can allocate.

This CL changes the evaluation of i==c to
	x, ok := i.(C); ok && x == c

The new generated code is shorter, uses less stack space,
does not allocate, and makes one runtime call instead of two.

This kind of comparison occurs in 38 places in the stdlib,
mostly in the net and os packages.

benchmark                     old ns/op     new ns/op     delta
BenchmarkEqEfaceConcrete      29.5          7.92          -73.15%
BenchmarkEqIfaceConcrete      32.1          7.90          -75.39%
BenchmarkNeEfaceConcrete      29.9          7.90          -73.58%
BenchmarkNeIfaceConcrete      35.9          7.90          -77.99%

Fixes golang#9370.

Change-Id: I7c4555950bcd6406ee5c613be1f2128da2c9a2b7
josharian added a commit to josharian/go that referenced this issue Dec 26, 2014
Consider an interface value i of type I and concrete value c of type C.

Prior to this CL, i==c was evaluated as
	I(c) == i

Evaluating I(c) can allocate.

This CL changes the evaluation of i==c to
	x, ok := i.(C); ok && x == c

The new generated code is shorter and does not allocate directly.

If C is small, as it is in every instance in the stdlib,
the new code also uses less stack space
and makes one runtime call instead of two.

If C is very large, the original implementation is used.
The cutoff for "very large" is 1<<16,
following the stack vs heap cutoff used elsewhere.

This kind of comparison occurs in 38 places in the stdlib,
mostly in the net and os packages.

benchmark                     old ns/op     new ns/op     delta
BenchmarkEqEfaceConcrete      29.5          7.92          -73.15%
BenchmarkEqIfaceConcrete      32.1          7.90          -75.39%
BenchmarkNeEfaceConcrete      29.9          7.90          -73.58%
BenchmarkNeIfaceConcrete      35.9          7.90          -77.99%

Fixes golang#9370.

Change-Id: I7c4555950bcd6406ee5c613be1f2128da2c9a2b7
josharian added a commit to josharian/go that referenced this issue Dec 29, 2014
Consider an interface value i of type I and concrete value c of type C.

Prior to this CL, i==c was evaluated as
	I(c) == i

Evaluating I(c) can allocate.

This CL changes the evaluation of i==c to
	x, ok := i.(C); ok && x == c

The new generated code is shorter and does not allocate directly.

If C is small, as it is in every instance in the stdlib,
the new code also uses less stack space
and makes one runtime call instead of two.

If C is very large, the original implementation is used.
The cutoff for "very large" is 1<<16,
following the stack vs heap cutoff used elsewhere.

This kind of comparison occurs in 38 places in the stdlib,
mostly in the net and os packages.

benchmark                     old ns/op     new ns/op     delta
BenchmarkEqEfaceConcrete      29.5          7.92          -73.15%
BenchmarkEqIfaceConcrete      32.1          7.90          -75.39%
BenchmarkNeEfaceConcrete      29.9          7.90          -73.58%
BenchmarkNeIfaceConcrete      35.9          7.90          -77.99%

Fixes golang#9370.

Change-Id: I7c4555950bcd6406ee5c613be1f2128da2c9a2b7
josharian added a commit to josharian/go that referenced this issue Jan 6, 2015
Consider an interface value i of type I and concrete value c of type C.

Prior to this CL, i==c was evaluated as
	I(c) == i

Evaluating I(c) can allocate.

This CL changes the evaluation of i==c to
	x, ok := i.(C); ok && x == c

The new generated code is shorter and does not allocate directly.

If C is small, as it is in every instance in the stdlib,
the new code also uses less stack space
and makes one runtime call instead of two.

If C is very large, the original implementation is used.
The cutoff for "very large" is 1<<16,
following the stack vs heap cutoff used elsewhere.

This kind of comparison occurs in 38 places in the stdlib,
mostly in the net and os packages.

benchmark                     old ns/op     new ns/op     delta
BenchmarkEqEfaceConcrete      29.5          7.92          -73.15%
BenchmarkEqIfaceConcrete      32.1          7.90          -75.39%
BenchmarkNeEfaceConcrete      29.9          7.90          -73.58%
BenchmarkNeIfaceConcrete      35.9          7.90          -77.99%

Fixes golang#9370.

Change-Id: I7c4555950bcd6406ee5c613be1f2128da2c9a2b7
josharian added a commit to josharian/go that referenced this issue Jan 9, 2015
Consider an interface value i of type I and concrete value c of type C.

Prior to this CL, i==c was evaluated as
	I(c) == i

Evaluating I(c) can allocate.

This CL changes the evaluation of i==c to
	x, ok := i.(C); ok && x == c

The new generated code is shorter and does not allocate directly.

If C is small, as it is in every instance in the stdlib,
the new code also uses less stack space
and makes one runtime call instead of two.

If C is very large, the original implementation is used.
The cutoff for "very large" is 1<<16,
following the stack vs heap cutoff used elsewhere.

This kind of comparison occurs in 38 places in the stdlib,
mostly in the net and os packages.

benchmark                     old ns/op     new ns/op     delta
BenchmarkEqEfaceConcrete      29.5          7.92          -73.15%
BenchmarkEqIfaceConcrete      32.1          7.90          -75.39%
BenchmarkNeEfaceConcrete      29.9          7.90          -73.58%
BenchmarkNeIfaceConcrete      35.9          7.90          -77.99%

Fixes golang#9370.

Change-Id: I7c4555950bcd6406ee5c613be1f2128da2c9a2b7
josharian added a commit to josharian/go that referenced this issue Feb 5, 2015
Consider an interface value i of type I and concrete value c of type C.

Prior to this CL, i==c was evaluated as
	I(c) == i

Evaluating I(c) can allocate.

This CL changes the evaluation of i==c to
	x, ok := i.(C); ok && x == c

The new generated code is shorter and does not allocate directly.

If C is small, as it is in every instance in the stdlib,
the new code also uses less stack space
and makes one runtime call instead of two.

If C is very large, the original implementation is used.
The cutoff for "very large" is 1<<16,
following the stack vs heap cutoff used elsewhere.

This kind of comparison occurs in 38 places in the stdlib,
mostly in the net and os packages.

benchmark                     old ns/op     new ns/op     delta
BenchmarkEqEfaceConcrete      29.5          7.92          -73.15%
BenchmarkEqIfaceConcrete      32.1          7.90          -75.39%
BenchmarkNeEfaceConcrete      29.9          7.90          -73.58%
BenchmarkNeIfaceConcrete      35.9          7.90          -77.99%

Fixes golang#9370.

Change-Id: I7c4555950bcd6406ee5c613be1f2128da2c9a2b7
@josharian josharian closed this in 77a2113 Feb 12, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.