Skip to content

runtime.closure() is being called for function literals that don't need it. #1894

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

Closed
gopherbot opened this issue May 28, 2011 · 10 comments
Closed
Milestone

Comments

@gopherbot
Copy link
Contributor

by fvbommel:

What steps will reproduce the problem?
1. Compile this program with -S:
=====
package foo

var Fn func(y int) int

func Foo() {
    Fn = func(y int) int { return y }
}
=====
2. Observe that Foo() calls runtime.closure

What is the expected output?
Foo() shouldn't need to call runtime.closure() since the function literal doesn't use
any of the variables of the outer function.

What do you see instead?
Foo() calls runtime.closure(0, _func_001), which unconditionally allocates a closure and
fills it with "call _func_001 (or movq _func_001, CX; call *CX); add 0, SP;
ret"

Which compiler are you using (5g, 6g, 8g, gccgo)?
6g

Which operating system are you using?
Ubuntu 10.10

Which revision are you using?  (hg identify)
c98449d685d2 weekly/weekly.2011-05-22
(But it happens with "7e539bf111cd tip" too)

Please provide any additional information below.
AFAICT, there are two ways to fix this:
1) The compiler could detect this (IIUC, in gc/closure.c:walkclosure()) and just use the
nested function directly instead of calling runtime.closure in this case.

2) The various runtime.closure() implementations (runtime/*/closure.c could detect this,
and just "if (siz == 0) return fn;"

IMHO, the first solution is preferable.
@adg
Copy link
Contributor

adg commented May 30, 2011

Comment 1:

Labels changed: added compilerbug.

Owner changed to @rsc.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented May 30, 2011

Comment 2:

An allocation is required because of the way == is defined
and implemented on func values.

@gopherbot
Copy link
Contributor Author

Comment 3 by fvbommel:

I'm not sure where the specification says that.
It just says that "Function values are equal if they refer to the same function or if
both are nil."
The section on function literals doesn't mention anything about necessarily creating
separate functions.
One could argue that two functions resulting from the same function literal (which
doesn't refer to local variables) refer to the same function since they can't be
distinguished by calling them: they have the same results and the same side-effects.
And they're written in the same place in the source...
It *would* lead to a subtle difference in semantics between closing and non-closing
function literals though, I suppose.

@bradfitz
Copy link
Contributor

Comment 4:

Now that http://code.google.com/p/go/source/detail?r=c987cc71cf15 is in, this could be
fixed.

@rsc
Copy link
Contributor

rsc commented Dec 9, 2011

Comment 5:

Labels changed: added priority-later.

@rsc
Copy link
Contributor

rsc commented Dec 12, 2011

Comment 6:

Labels changed: added priority-go1.

@lvdlvd
Copy link

lvdlvd commented Dec 13, 2011

Comment 7:

Owner changed to @lvdlvd.

@lvdlvd
Copy link

lvdlvd commented Jan 6, 2012

Comment 8:

Status changed to Started.

@lvdlvd
Copy link

lvdlvd commented Jan 6, 2012

Comment 9:

Pending in  http://golang.org/cl/5516051

@lvdlvd
Copy link

lvdlvd commented Jan 10, 2012

Comment 10:

This issue was closed by revision ba25778.

Status changed to Fixed.

@rsc rsc added this to the Go1 milestone Apr 10, 2015
@rsc rsc removed the priority-go1 label Apr 10, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants