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

sync/atomic: improve package docs #18955

Closed
yaxinlx opened this issue Feb 6, 2017 · 4 comments
Closed

sync/atomic: improve package docs #18955

yaxinlx opened this issue Feb 6, 2017 · 4 comments
Labels
Documentation FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@yaxinlx
Copy link

yaxinlx commented Feb 6, 2017

At the end of the API docs of sync/atomic, https://golang.org/pkg/sync/atomic/, it says:

On both ARM and x86-32, it is the caller's responsibility to arrange for 64-bit alignment of 64-bit words accessed atomically. The first word in a global variable or in an allocated struct or slice can be relied upon to be 64-bit aligned.

It could be improved as

The first word in a variable or in an allocated struct, array or slice can be relied upon to be 64-bit aligned.

The reason is there are many local variables and arrays which addresses are passed as the parameters atomic 64bit functions, in go project source.

And "the first word in an allocated slice" is not an accurate description, it should be the first element in an allocated slice which elements are 64-bit words.

And the expvar package docs should warning that expvar.Int and expvar.Float are not safe to be embedded in other types on 32bit OSes.

Please see this thread for detail: https://groups.google.com/forum/#!topic/golang-nuts/_nHK6P8_lhw

@bradfitz bradfitz changed the title documents: improve API docs sync/atomic: improve package docs Feb 6, 2017
@bradfitz bradfitz added this to the Go1.9Maybe milestone Feb 6, 2017
@bradfitz bradfitz added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jun 28, 2017
@bradfitz
Copy link
Contributor

@aclements, do you want to improve these docs for Go 1.9?

@aclements
Copy link
Member

The first word in a variable or in an allocated struct, array or slice can be relied upon to be 64-bit aligned.

I think this is fair.

And "the first word in an allocated slice" is not an accurate description, it should be the first element in an allocated slice which elements are 64-bit words.

Can you expand on why you think this isn't an accurate description? It seems accurate to me.

(Really, this whole statement is somewhat inaccurate, as it's only true if the allocation itself is at least 8 bytes. But if that's not the case, then there's no way to use the 64-bit atomic ops anyway.)

@gopherbot
Copy link

CL https://golang.org/cl/48112 mentions this issue.

@go101
Copy link

go101 commented Jul 12, 2017

A slice is composed a header and an underlying array.
When we say "an allocated slice", we generally mean the header of the slice is allocated.
But we would never do 64-bit atomic operations on slice headers.
And for an allocated slice, its underlying array may be not allocated.
For example, its underlying array may be a field of a struct value.

And "the first word in an allocated slice" is not an accurate description, it should be the first element in an allocated slice which elements are 64-bit words.

The above texts may be also not good. I prefer to use the following one:

The first word in a variable or in an allocated struct and array can be relied upon to be 64-bit aligned. The elements of a slice are stored in an underlying array of the slice.

@golang golang locked and limited conversation to collaborators Jul 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants