-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
Comments
Another go/types vs cmd/compile disagreement: go/types thinks |
@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:
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. |
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. |
@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). |
unsafe.Sizeof
andtypes.StdSizes.Sizeof
disagree about the size ofstruct { x int32; y [0]byte }
: http://play.golang.org/p/D-YJMj-Xd0See also issue #9401.
The text was updated successfully, but these errors were encountered: