On Sat, Feb 4, 2017 at 11:13 PM, Ian Lance Taylor <i...@golang.org> wrote:
> On Sat, Feb 4, 2017 at 7:33 AM, Axel Wagner
> <axel.wagner...@googlemail.com> wrote:
> >
> > I believe, that fields do not necessarily need to be properly aligned
> > (explaining 599). However, *structs* have a necessary alignment, as
> required
> > by their fields. So, if you do
> > type S struct {
> > A uint32
> > B uint64
> > }
> > then there is no guarantee, that B is 64-bit aligned. The spec tells us,
> > that unsafe.Alignof(s.A) will return 4 ("The functions Alignof and Sizeof
> > take an expression x of any type and return the alignment or size,
> > respectively, of a hypothetical variable v as if v was declared via var
> v =
> > x", s.A is of type uint32 and such a variable has alignment 4),
> > unsafe.Alignof(s.B) will return 8 and thus, unsafe.Alignof(s) will
> return 8
> > ("For a variable x of struct type: unsafe.Alignof(x) is the largest of
> all
> > the values unsafe.Alignof(x.f) for each field f of x, but at least 1").
>
> The spec does not say that unsafe.Alignof(s.B) will return 8. In
> fact, on 32-bit targets, with the gc toolchain, it will return 4.
>
To me, that seems to contradict the combination of "The functions Alignof
and Sizeof take an expression x of any type and return the alignment or
size, respectively, of a hypothetical variable v as if v was declared via
var v = x" and the table at the very bottom, saying an uint64 has Alignof
8. The latter seems to strongly imply that a "var v = s.B" must be 8-byte
aligned, as v is a variable (even an allocated one; one of the few mentions
of "allocate" in the spec :) ) of type uint64. Then, the former would imply
that the same happens to s.B, as that is how the function is defined.
I think the spec and docs do provide enough information to use
> sync/atomic correctly. "The first word in a global variable or in an
> allocated struct or slice can be relied upon to be 64-bit aligned."
>
In that context, the original question in this thread was "what does
'allocated' mean and I find that more and more a fair question" :)
> That is sufficient to ensure that your code works correctly. It does
> mean that certain kinds of operations don't work. sync.WaitGroup
> plays the games it does to avoid introducing an extra allocation.
>
I don't understand. Where is the extra allocation, if you where to change
state to be an uint64 and just dereference it, instead of using
unsafe.Pointer casting in state()?
Am I understanding it correctly that you are saying a) yes, fields like,
say expvar.Float can not be embedded, unless you take special care
(recursively) of having it aligned and b) the only safe way to use atomic
with fields would be, to put a pointer in and allocate with new (and that's
the "extra allocation" WaitGroup is trying to avoid?).
I would find that situation understandable, but also pretty unfortunate. It
seems very easy to make a mistake that way (embedding a type, not knowing
that it relies on atomics). It also means, it's harder to make the zero
value of a struct useful; while having a uint64 intiialized to zero is a
perfectly fine default for a lot of cases, needing to allocate it with new
requires a constructor.
I would hope we could at least a) recommend that (and maybe the WaitGroup
trick) as "the" ways to use atomic somehow and b) have some kind of check
to detect misuse. The memory layout of all structs is statically known, so
it should be theoretically possible to detect usages of
atomic.AddUint64(&s.f) (or the like) with a misaligned field, no?
> There is probably something to be fixed here, to permit higher
> efficiency without requiring trickiness like that of sync.WaitGroup,
> but I'm not sure how it should be implemented.
>
> Ian
>
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
