golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124k stars 17.67k forks source link

sync/atomic: support int/uint types #5721

Open dvyukov opened 11 years ago

dvyukov commented 11 years ago
Add operations on int/uint types to sync/atomic package.

int is used as indices/len/cap by slices/maps/chans, widely used in
std lib interfaces and seems to be the "default" integer type
otherwise.
Current atomic interface forces to use casts to work with int type.
Casts are ugly.
There is another unpleasant implication -- the casts make code less
portable because force you to say explicitly how many bits you expect
int is.
If int64 is used instead of int for the sake of atomic operations, it incurs performance
penalty on 32-bit platforms, and raises alignment issues.
minux commented 11 years ago

Comment 1:

do we need uint?
i think using uintptr is better.
dvyukov commented 11 years ago

Comment 2:

uint is mostly for completeness.
Somebody may extensively use uint due to personal preferences. Then it will require
casts again.
dsymonds commented 11 years ago

Comment 3:

I agree we should have uint for completeness.
rsc commented 11 years ago

Comment 4:

If you are playing atomic games, I think you should be forced to think about word size.
The number of times where you can use atomics to store just one value of unspecified
size is small.

Status changed to Thinking.

rsc commented 11 years ago

Comment 5:

If you are playing atomic games, I think you should be forced to think about word size.
The number of times where you can use atomics to store just one value of unspecified
size is small.
dsymonds commented 11 years ago

Comment 6:

You may have thought about word size and realised that int is more
portably efficient between 32-bit and 64-bit than int32 or int64.
dvyukov commented 11 years ago

Comment 7:

>The number of times where you can use atomics to store just one value of unspecified
size is small.
I disagree.
While int has unspecified size, it has "sufficiently large" size since it's used as e.g.
slice len.
An examples are:
- assign unique ids to goroutines
- count number of pending goroutines/work items
- atomic flag (0/1), e.g. "stop flag" or "has errors"
rsc commented 11 years ago

Comment 8:

I'm incredibly skeptical. Give me a real example where you really need int
instead of int32 or int64.
dsymonds commented 11 years ago

Comment 9:

I wouldn't say "need", but there's been several occasions where I've
made a struct field be an int32 simply because I wanted to do atomic
operations on it. If there weren't atomics (e.g. if I had used a
mutex) then they would have been ints.
dvyukov commented 11 years ago

Comment 10:

What do you mean by "really need"? Where do I really need int to store an index in a
slice instead of int32/64?...
rsc commented 11 years ago

Comment 11:

I have no problem with atomic ints in the runtime. I don't understand why
they need to be in sync/atomic.
I think it is safe to assume that there are no more than active 2^31-1
goroutines in a program.
To assign unique IDs, you'd want an int64 to avoid reuse, even on a 32-bit
system.
To count number of pending goroutines, an int32 seems fine.
For an atomic flag, an int32 seems better than an int: you avoid wasting
space on 64-bit systems.
Atomics make it harder to reason about the correctness of programs. Being
required to specify the size of the atomic word at least removes one
variable from the correctness decision.
Also, I like the fact that it's inconvenient to use atomics. It should be.
I especially like David's comment, about not having atomic ints making him
think about using mutexes instead. That's exactly the behavior we want to
encourage.
dvyukov commented 11 years ago

Comment 12:

int is needed when it refers to an index in a slice.
int is at least as needed as all signed atomic operations (or all unsigned) and uintptr
(or unsafe.Pointer).
What is "really needed" is 1 type uint64. Everything else is expressible through it with
some amount of casts and assumptions.
rsc commented 11 years ago

Comment 13:

This is all still very abstract. 
Until I see real code demonstrating the need, where using int32 or int64 is obviously
worse, no.
dvyukov commented 11 years ago

Comment 14:

I've looked at vitess source for cases where atomic variables refer to slice/map/chan
len/cap.
https://code.google.com/p/vitess/source/browse/go/pools/resource_pool.go
capacity    sync2.AtomicInt64
represents chan capacity
https://code.google.com/p/vitess/source/browse/go/streamlog/streamlog.go
size sync2.AtomicUint32
represents map size
https://code.google.com/p/vitess/source/browse/go/vt/tabletserver/query_engine.go
        streamBufferSize sync2.AtomicInt32
represent sum of slice len's
In lots of other cases which do not require huge vals (e.g. unique ids or total bytes
read), int would be just more natural and remove casts because all other types around
are int's.
rsc commented 11 years ago

Comment 15:

Labels changed: added feature.

rsc commented 11 years ago

Comment 16:

I am still skeptical. Let's leave this out. We can revisit for next time.

Labels changed: added go1.3maybe, removed go1.2maybe.

robpike commented 11 years ago

Comment 17:

Labels changed: removed go1.3maybe.

rsc commented 10 years ago

Comment 18:

Labels changed: added go1.3maybe.

rsc commented 10 years ago

Comment 19:

Labels changed: removed feature.

rsc commented 10 years ago

Comment 20:

Labels changed: added release-none, removed go1.3maybe.

rsc commented 10 years ago

Comment 21:

Labels changed: added repo-main.

arkadijs commented 8 years ago

Here is the code: sequentially numbering files while uploading to AWS S3 from multiple goroutines.

func s3UploadWorker(wg *sync.WaitGroup, fileCounter *int32, basename string, pipe <-chan [][]byte, s3 *awss3.S3) {
    for blobs := range pipe {
        if len(blobs) == 0 {
            break
        }
        buffer, gz := s3GzippedBuffer()
        for _, blob := range blobs {
            gz.Write(blob)
        }
        gz.Close()
        currentFileN := atomic.AddInt32(fileCounter, 1)
        uploadToS3(s3, basename, int(currentFileN), buffer)
    }
    wg.Done()
}
vearutop commented 7 years ago

How can I change exported int variable of 3rd party library which is being read concurrently?

package lib
var A int

This is how I wanted to do:

package myapp
import "path.to/lib"
func SetA(v int) {
    atomic.StoreInt(&lib.A, v)
}

What is a proper way to do if I don't control 3rd party code?

ALTree commented 7 years ago

@vearutop Go project does not use its bug tracker for general discussion or asking questions about the language. The Github bug tracker is only used for tracking bugs and proposals going through the Proposal Process.

Please see the Questions wiki page; it has a list of good places for asking questions. Thanks!

AnyCPU commented 5 years ago

int would be just more natural and remove casts because all other types around are int's.

Yeah, it sounds reasonable even in 2019. That's why I'm going to do casting.

hodgesds commented 4 years ago

The Linux kernel has a new io_uring API that it would be real handy to have int support as slices are mmap'd for submitting/completing IO requests.

slrz commented 4 years ago

Could you expand on that, please? Do you mean updating slice lengths using unsafe?

In general, things like io_uring make it even more expedient to be explicit and think about the exact quantity you want to update as the kernel's idea of an int might be very different from Go's. Looks more like a reason to not add int/uint functions to sync/atomic.