atomic operations and chan

[Sokolov Yura]

Hello, all.

There is atomic.StorePointer and atomic.LoadPointer, which could be used for... it is obvious what for

`chan` and `map` are internally just a pointers, but there is not way to convert them to and from unsafe.Pointer :-(

for example, i have a chan in a struct

    type S struct {

       c chan struct{}

    }

and I want to atomically load pointer to `c`

    func (s *S) Chan() chan struct {} {

        // I can load pointer

        c := atomic.LoadPointer((*unsafe.Pointer)(unsafe.Pointer(&s.c)))

        // but I cannot convert it to chan

        // return (chan struct{})(c)

    }

How can I do it?

Is `chan` always (will be) represented as a pointer to some opaque structure?

-------

Sokolov Yura aka funny_falcon

[Nate Finch]

You should tell us about the problem you're trying to solve before asking about the solution.  Even if what you're asking is possible (and I don't think it is), it's probably a bad idea.

What are you trying to do?  maybe we can help you find a better way to solve your problem that doesn't require atomic at all.

[Юрий Соколов]

I'm trying to save a bit of space.
I'm making future with channel that is closed when future fulfilled.

// code is simplified
type Future struct {
m sync.Mutex
c chan struct{}
v interface{}
}

func (f *Future) Fill(v interface{}) {
f.m.Lock()
if f.v == nil {
f.v = v
close(f.c)
// now f.c is not distinguishable from any other closed
`chan struct{}`
// so it is possible to replace it with global one, to
allow GC collect f.c
f.c = global_closed
// unfortunately Go race detector blames line above :-(
}
f.m.Unlock()
}

var global_closed = make(chan struct{})
func init() { close(global_closed); }

I've already did fetching `f.c` under mutex, looks like it doesn't
slows thing much,
but I'd rather use atomic.LoadPointer/StorePointer if it is possible.

> --
> You received this message because you are subscribed to a topic in the
> Google Groups "golang-nuts" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/golang-nuts/lIws4R3mpsQ/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> golang-nuts...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.



--
With regards,
Sokolov Yura aka funny_falcon

[Alex Bligh]

Is your mutex protecting every use of f.c?

If not, the above code is unsafe because a use of f.c (elsewhere)
could be halfway through loading f.c when
f.c = global_closed
is run, causing corruption, and the race detector is correct.

Note that in the general case assigning a 'global closed' channel
does not give the same result.

My understanding is that closing a channel is done by the writer,
and there could still be things in the channel yet to be read
by the reader. By assigning global_closed to it, those things will
be lost. That may not be the case here with an unbuffered
channel of struct{}. But with an unbuffered channel of struct{}
I'm not quite sure what the 'bit of space' you are trying to
save is?

--
Alex Bligh

[Jakob Borg]

2016-04-26 19:58 GMT+02:00 Юрий Соколов <funny....@gmail.com>:
> I'm trying to save a bit of space.

You shouldn't worry about the space used for a chan struct{}.
Certainly not to the extent of doing atomic or unsafe trickery.

//jb

[Sokolov Yura]

You understand wrong.

Channel is used only to signal waters, that value is already set.
Channel itself does not contain any value. It is just "broadcaster".

That is why closed channel (in my use case) is not distinguishable 

from other closed channel.

вторник, 26 апреля 2016 г., 21:08:37 UTC+3 пользователь Alex Bligh написал:

If not, the above code is unsafe because a use of f.c (elsewhere)
could be halfway through loading f.c when
  f.c = global_closed
is run, causing corruption, and the race detector is correct.

And you again not quite right.
`chan` is represented as a pointer to some opaque struct.
So `f.c = global_closed` is just a write of pointer.

On x86 and x86_64 write to aligned pointer sized place is atomic,

so here is no real race.

And it is really possible to use atomic.LoadPointer/StorePointer.

In fact, I already did the trick, and it works, and race detector happy.

But code is ugly and not faster than wrapping with mutex :-(

I suppose, code is slower cause it is ugly, and Go's optimizer does no job well.

 

--
Alex Bligh

[Sokolov Yura]

Jakob, sizeof channel structure is at least 96 byte.
Real Future struct (not this simplified) is 56 byte (+ channel)

Given average value result is 128 byte, to let channel go is to safe 33% memory.

вторник, 26 апреля 2016 г., 21:17:11 UTC+3 пользователь Jakob Borg написал:

[Thomas Bushnell, BSG]

This only makes sense if this is an important category of memory consumption for your program. Do you know that it is? How many of these objects do you have?

--
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...@googlegroups.com.

[Alex Bligh]

On 26 Apr 2016, at 19:20, Sokolov Yura <funny....@gmail.com> wrote:

> You understand wrong.
>
> Channel is used only to signal waters, that value is already set.
> Channel itself does not contain any value. It is just "broadcaster".
> That is why closed channel (in my use case) is not distinguishable
> from other closed channel.

I understand that perfectly, and as I said, in that case I don't
see what material space you are saving.

> вторник, 26 апреля 2016 г., 21:08:37 UTC+3 пользователь Alex Bligh написал:
>
> If not, the above code is unsafe because a use of f.c (elsewhere)
> could be halfway through loading f.c when
> f.c = global_closed
> is run, causing corruption, and the race detector is correct.
>
>
> And you again not quite right.
> `chan` is represented as a pointer to some opaque struct.
> So `f.c = global_closed` is just a write of pointer.
> On x86 and x86_64 write to aligned pointer sized place is atomic,
> so here is no real race.

I believe you are incorrect.

go today on the compiler you are using today might compile
f.c = global_closed as something which
on some architectures is atomic, but it provides no guarantee
of it. Tomorrow it might not. It might choose to store byte by
byte. Or store an intermediate value and then the real value.
You have no guarantee, and that's what the race detector is
correctly flagging up.

The race detector does not detect things on the basis that they
definitely will cause issues on your architecture with your
current compiler. Rather it detects situations where the
behaviour given is undefined, in the sense of not protected
by go's contract.

> And it is really possible to use atomic.LoadPointer/StorePointer.
> In fact, I already did the trick, and it works, and race detector happy.

Sure. Using atomic.StorePointer will give you the guarantee you
(and the race detector) are seeking.

> But code is ugly and not faster than wrapping with mutex :-(
> I suppose, code is slower cause it is ugly, and Go's optimizer does no job well.

By far the least ugly route would be to not use global_closed.

--
Alex Bligh

[Roberto Zanotto]

You should be able to atomically load and store channel and map values inside atomic.Value and avoid unsafe altogether.

https://golang.org/pkg/sync/atomic/#Value

[Sokolov Yura]

вторник, 26 апреля 2016 г., 21:26:05 UTC+3 пользователь Alex Bligh написал:

On 26 Apr 2016, at 19:20, Sokolov Yura <funny....@gmail.com> wrote:

> You understand wrong.
>
> Channel is used only to signal waters, that value is already set.
> Channel itself does not contain any value. It is just "broadcaster".
> That is why closed channel (in my use case) is not distinguishable
> from other closed channel.

I understand that perfectly, and as I said, in that case I don't
see what material space you are saving.

96 bytes per channel that can be reused after GC. size of channel is 96 byte.

 

go today on the compiler you are using today might compile 

f.c = global_closed as something which
on some architectures is atomic, but it provides no guarantee
of it. Tomorrow it might not. It might choose to store byte by
byte. Or store an intermediate value and then the real value.
You have no guarantee, and that's what the race detector is
correctly flagging up.

You are right, channel "handle" may change in theory.
In practice, it is very doubtfully, cause you are free to copy channel handle,

and it preserves its identity with copying (like map and unlike slice).

So, it is almost safe to presume, that `chan` will always be just a pointer to

some opaque struct.

 

> But code is ugly and not faster than wrapping with mutex :-(
> I suppose, code is slower cause it is ugly, and Go's optimizer does no job well.

By far the least ugly route would be to not use global_closed.

I'm not agree.
At least, current approach with reusing mutex works well.

Mutex already should be hold when value is setting.

Taking mutex while fetching channel doesn't cost much, cause it happens rare and usually does not contend.

But I always think, not taking mutex is better than taking mutex.

So, if language allowed me to use atomic directly, i'd rather use that.

 

--
Alex Bligh

--

Sokolov Yura aka funny_falcon 

[Sokolov Yura]

Roberto, you are right.

Unfortunately, code should work with go1.3 :-(

вторник, 26 апреля 2016 г., 21:34:40 UTC+3 пользователь Roberto Zanotto написал:

[Sokolov Yura]

Thomas, you are almost right.

But it should be "general library" code, so it shall behave well in every situation, without presumptions.

вторник, 26 апреля 2016 г., 21:26:01 UTC+3 пользователь Thomas Bushnell, BSG написал:

[Sokolov Yura]

вторник, 26 апреля 2016 г., 21:26:05 UTC+3 пользователь Alex Bligh написал:

go today on the compiler you are using today might compile
f.c = global_closed as something which
on some architectures is atomic, but it provides no guarantee
of it. Tomorrow it might not. It might choose to store byte by
byte. Or store an intermediate value and then the real value.
You have no guarantee, and that's what the race detector is
correctly flagging up.

On a second try, I think you are right:
channel may be implemented like interface, ie with couple of pointers.

It is still doubtfully to happen, but it has sense.

So that, reusing mutex is only safe approach.

    func (f *Future) Chan <-chan struct{} {

        f.m.Lock()

        c := f.c

        f.m.Unlock()

        return c

    }

 

--
Alex Bligh

[Thomas Bushnell, BSG]

On Tue, Apr 26, 2016 at 11:41 AM Sokolov Yura <funny....@gmail.com> wrote:

Thomas, you are almost right.

But it should be "general library" code, so it shall behave well in every situation, without presumptions.

I don't know what "general library" code is. All code makes assumptions. The thing to do is to care about the most frequent cases, in general. It is very rare that anyone should be caring about a few dozen bytes.

[Юрий Соколов]

That is untill you have millions of few dozens bytes.

26 апр. 2016 г. 22:37 пользователь "Thomas Bushnell, BSG" <tbus...@google.com> написал:

[Thomas Bushnell, BSG]

You have limited time to improve things.

Why do you believe that *this* is the key library to spend energy on microscopic improvements rather than some other?

How much time are you prepared to spend debugging data races because you saved a small amount of memory here?

Thomas

[Dan Kortschak]

Two ways:

http://play.golang.org/p/0YMzijuizE


The merits of doing either of these things for any particular purpose are not commented on here.

[Юрий Соколов]

26 апр. 2016 г. 11:54 PM пользователь "Dan Kortschak" <dan.ko...@adelaide.edu.au> написал:

>
> Two ways:
>
> http://play.golang.org/p/0YMzijuizE
>
>
> The merits of doing either of these things for any particular purpose are not commented on here.

Cast to unsafe.Pointer is obvious.

Harder to cast pointer back to `chan` and `map`.

[Юрий Соколов]

I've already found a way to cast it back.
But code is ugly, and there other simpler workarounds exist.

26 апр. 2016 г. 11:58 PM пользователь "Юрий Соколов" <funny....@gmail.com> написал:

[Dan Kortschak]

It's no uglier than any other unsafe code

http://play.golang.org/p/t7JCm3wOCs

[George Burgess IV]

Have you considered using a sync.WaitGroup instead? They take a total of 16 bytes each (as implemented here: https://golang.org/src/sync/waitgroup.go ), and it looks like WaitGroup.Wait() is substantially simpler (read: probably faster) than the channel recv code (https://github.com/golang/go/blob/master/src/runtime/chan.go#L393).

[Юрий Соколов]

Main intention were to have ability to use `select` statement.
Unfortunately, there is no ability to select from WaitGroup.

27 апр. 2016 г. 5:37 пользователь "George Burgess IV" <george.b...@gmail.com> написал: