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

go/types: StdSizes does not account for padding for trailing zero size fields #12886

Closed
mdempsky opened this issue Oct 9, 2015 · 4 comments
Closed

Comments

@mdempsky
Copy link
Member

mdempsky commented Oct 9, 2015

unsafe.Sizeof and types.StdSizes.Sizeof disagree about the size of struct { x int32; y [0]byte }: http://play.golang.org/p/D-YJMj-Xd0

See also issue #9401.

@mdempsky
Copy link
Member Author

mdempsky commented Oct 9, 2015

Another go/types vs cmd/compile disagreement: go/types thinks struct { x int64; y int32 } is size 12 (for StdSizes{WordSize: 8, MaxAlign: 8}, when cmd/compile says it's size 16 (on GOARCH=amd64p32): http://play.golang.org/p/L13xQSR1Iy

@griesemer
Copy link
Contributor

@mdempsky Can you explain to me why this matters for go/types, i.e., why it should be the same as what gc provides (independent of issue #9401)? This is the unsafe package, and also we don't guarantee compatibility.

The go/types size computation also differs in other ways: For a struct, it always computes the minimal size (never with padding). If padding is needed for alignment if the struct is used in an array, or as a field in another struct, that padding is added then. This can save significant amount of space. For instance, in this program:

package main

import (
    "unsafe"
)

type T1 struct {
    a float64
    b byte
}

type T2 struct {
    a T1
    b byte
}

func main() {
    var t1 T1
    println(unsafe.Sizeof(t1)) // gc: 16
    var t2 T2
    println(unsafe.Sizeof(t2)) // gc: 24
}

gc returns 16 for the size of T1 even though it truly only needs 9 bytes. As a result, the size of T2 is 24 bytes even though it would be fine with just 10 bytes (and it would not violate any alignment issues). It's easy to construct types that use much more space in gc than would be necessary (w/o violating alignment) if those were used as elements of an array.

Or in other words, the Sizeof computation by go/types always returns the minimum actual space required. If there's implementation requirements (alignment, GC) that require padding, that needs to be added separately.

@mdempsky
Copy link
Member Author

Perhaps it was just mistaken expectations. For github.com/mdempsky/maligned, I wanted to measure the size of structs as they'd be implemented by cmd/compile, and I thought StdSizes was meant to be usable by users for that purpose.

However, if StdSizes is just meant to be a valid types.Sizes implementation and intentionally not expected to match gc and/or gccgo's semantics, then I think it's reasonable to close as working as intended.

@griesemer
Copy link
Contributor

@mdempsky It's not intentionally not expected to match, but I've also not tried to match it - maybe it should, but I haven't seen a convincing reason for it. As my example above shows, the gc implementation (and I believe gccgo as well, to some extent) is not making a difference between padding needed when composing types, and actual sizes. It simply assumes that all types' sizes are the sizes after padding (needed or not); and of course the built-in types are carefully designed/chosen to not require internal padding.

Thus, gc-compiled structs that are not carefully laid out waste memory (internal fragmentation) - in some cases it cannot even be avoided with careful layout (if you don't own the struct).

In my view, that's why there's two functions: one for size, and one for alignment. The two together determine the size of composed types, exactly the same way as the two together are used to determine the size of a single struct (by aligning each field individually and thus determine padding between fields).

(I have discussed this in the past w/ others (iant). The argument for gc and gccgo was that the current solution is simpler and perhaps less surprising. Personally I don't agree. Again, to revive the struct example: When we ask for the size of a byte we expect to get 1, but inside a struct, the actual space used may be 2, 4, or 8 because the next field must be aligned. Nobody is surprised by that, and it would be rather unexpected to see a size of 4 or 8 for bytes just because that's the minimal alignment on a platform. I think the same should be true for composed types such as structs and arrays: The size should not include padding. Or in other words: We have a special case for built-in types in gc where there doesn't need to be one. go/types doesn't special case.)

Regarding issue #9401: We may still add extra space for an empty struct field at the end of a struct, but again, that shouldn't affect Sizeof, for the same reasons mentioned above. For one, if one were to copy memory byte-wise, padding doesn't need to be copied.

I'm going to close this for now. Feel free to re-open if you think there are good reasons to change the status quo for go/types besides strict compatibility when it comes to sizes (I'd rather have a gc specific size function instead).

@golang golang locked and limited conversation to collaborators Oct 9, 2016
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

3 participants