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

reflect: Type.Field allocates #2320

Closed
rsc opened this issue Sep 29, 2011 · 12 comments
Closed

reflect: Type.Field allocates #2320

rsc opened this issue Sep 29, 2011 · 12 comments
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Sep 29, 2011

it's the only thing in Type that allocates.  avoid.

    f.Index = []int{i}
@rsc
Copy link
Contributor Author

rsc commented Dec 9, 2011

Comment 1:

Labels changed: added priority-later, removed priority-medium.

@rsc
Copy link
Contributor Author

rsc commented Dec 12, 2011

Comment 2:

Labels changed: added priority-go1.

@remyoudompheng
Copy link
Contributor

Comment 3:

Why is StructField.Index a slice ? It would make sense if FieldByIndex([]int) would
actually return the same slice argument in the Index field, but it seems it is not the
case and it always returns a slice of length 1 there.
Is it for embedded structs ?

@rsc
Copy link
Contributor Author

rsc commented Dec 14, 2011

Comment 4:

Yes, it is because FieldByName returns a Field with a slice in it.

@robpike
Copy link
Contributor

robpike commented Jan 13, 2012

Comment 5:

Owner changed to builder@golang.org.

@bradfitz
Copy link
Contributor

Comment 7:

http://golang.org/cl/5564066

Owner changed to @bradfitz.

Status changed to Started.

@bradfitz
Copy link
Contributor

Comment 8:

Also, Russ's earlier version with read-only memory:
http://golang.org/cl/5371098
Pushing out past Go1.  No super compelling argument for it.

Labels changed: added priority-later, removed priority-go1.

Owner changed to builder@golang.org.

@rsc
Copy link
Contributor Author

rsc commented Sep 12, 2012

Comment 9:

Labels changed: added go1.1.

@rsc
Copy link
Contributor Author

rsc commented Dec 10, 2012

Comment 10:

Labels changed: added size-m.

@rsc
Copy link
Contributor Author

rsc commented Mar 12, 2013

Comment 11:

Per discussion on https://golang.org/cl/5371098, we've decided the drawbacks
outweigh the benefits. If the pendulum ever swings back, the CL is there.

Status changed to WorkingAsIntended.

@rsc rsc added this to the Go1.1 milestone Apr 14, 2015
@rsc rsc removed the go1.1 label Apr 14, 2015
@bprosnitz
Copy link
Contributor

Just want to leave a comment on this. This makes up about a third of the allocations for my team's encoder and decoder and has a major impact on performance. I looked into alternative strategies to solve this in type.go, but concluded that it isn't possible to have a satisfying solution so long as the index field is a slice and wanted to leave a note.

It isn't possible to back the slice by the stack (say by an unexposed array in the struct) because escape analysis (correctly) will detect that due to the return, the backing array of the slice must be on the heap. Once the slice must be backed by the heap, the only way to reduce allocations is to reduce the number of them. This isn't possible because we would either back the slice by constants (which breaks the semantics of being able to modify the returned value) or back it by an array that is shared across method calls (which is bad for a number of reasons).

@minux
Copy link
Member

minux commented Dec 19, 2015

Backing the slice with an unexported array in the struct won't work,
this has been suggested in https://golang.org/cl/5371098.

I think writable poitners to read-only memory are a bad idea. The best
idea I can have is make StructField.Index a slice of a hidden
StructField.index [4]int so that it doesn't escape and cover most cases.
This doesn't work: assignment of a StructField value will not update the
slice, and now you have a slice pointing back into the original StructField.

@golang golang locked and limited conversation to collaborators Dec 29, 2016
gopherbot pushed a commit that referenced this issue Sep 10, 2024
Use assembler to make runtime.staticuint64s into a readonly array
so that the reflect package can safely create a slice without requiring
any allocation.

Fixes #2320
Fixes #68380

Change-Id: If2c97238eca782d0632db265c840581d4ecb9d18
Reviewed-on: https://go-review.googlesource.com/c/go/+/597855
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
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

7 participants