FWIW, here's the pattern I've settled on for my actual application.
https://play.golang.org/p/OBVsp-Rjknf
It uses sync.Mutex only and includes the mutex as a member of the
guardedState struct. The deciding factor was that my industrial control
application has a very large state struct with over 500 members. Using a
mutex instead of atomic.Value permits me to pass the address of the g.v
directly to the update function.
// directUpdate is an order of magnitude faster than update for very large
// structs because it avoids 2 struct copies. Use it if the update
function, f,
// can be made error-proof.
func (g *guardedState) directUpdate(f func(*state)) {
g.mu.Lock()
defer g.mu.Unlock()
f(&g.st)
}
For cases where f cannot be made error-proof, I've kept the
copy/modify/store pattern of the original update method and added some
error-handling, thus:
// update applies an update function, f, to a copy of the state struct, and
// saves the updated copy back to g.v unless f returns error. If f returns
an
// error, leaves g.v unchanged.
func (g *guardedState) update(f func(*state) error) (err error) {
g.mu.Lock()
defer g.mu.Unlock()
s := g.st
err = f(&s)
if err != nil {
return
}
g.st = s
return
}
With a 500 member struct, I got the following comparative benchmarks:
BenchmarkUpdate-8 2000000 964 ns/op
BenchmarkDirectUpdate-8 20000000 60.2 ns/op
I appreciate the helpful discussions. Thank you, Skip, Matt, and Manilo.
On Sunday, April 14, 2019 at 7:47:41 PM UTC-4, Skip wrote:
>
> I believe you only need one or the other; I assumed a use case like this:
>
> https://play.golang.org/p/nSgyiXU87XN
>
>
> On Sun, Apr 14, 2019 at 1:33 PM <[email protected] <javascript:>> wrote:
>
>> I found a good discussion in the FAQ that explains the problem clearly.
>> https://golang.org/doc/faq#methods_on_values_or_pointers
>>
>> I think the mutex is needed on the update function with or without the
>> use of sync/atomic. The atomic guards the Load and the Store but the func
>> is operating on the copy returned by Load. Seems to me it's vulnerable to
>> being pre-empted unless the entire read/modify/write sequence is guarded by
>> a mutex.
>>
>> On Sunday, April 14, 2019 at 2:20:02 PM UTC-4, Skip wrote:
>>>
>>> I don't know if it's documented or not. In the language reference you
>>> can see the rules for method calls:
>>>
>>> https://golang.org/ref/spec#Calls
>>> https://golang.org/ref/spec#Method_sets
>>>
>>> A hint might have been that object should have mutated, but it didn't.
>>> It's in a class of errors that becomes recognizable once you've been bitten
>>> by it.
>>>
>>> In the example you gave, use of mutex seems redundant to me; execution
>>> of func passed into update is already guarded by atomic.
>>>
>>> On Sun, Apr 14, 2019 at 9:51 AM <[email protected]> wrote:
>>>
>>>> Thanks, Skip. That fixes it. Is the need for a pointer receiver
>>>> documented somewhere? It's not something that even crossed my mind given
>>>> that the neither the compiler nor golint complained. I suppose it makes
>>>> sense if I think of func (x) foo(y) {} as being an alternate way of
>>>> writing
>>>> func foo(x, y) {}. In that case, it's clear that a copy of x is being
>>>> passed since that's Go's default.
>>>>
>>>> While this topic is still alive, I'd like to ask a follow-on question:
>>>> Is the use of sync/atomic actually needed in this example or is it
>>>> sufficient to wrap all accesses in mutex Lock/Unlock (using the same
>>>> mutex,
>>>> of course).
>>>>
>>>>
>>>>
>>>> On Sunday, April 14, 2019 at 12:08:55 PM UTC-4, Skip wrote:
>>>>>
>>>>> The receiver for load and update should be the original object not a
>>>>> copy.
>>>>>
>>>>> https://play.golang.org/p/XCZC0OVhGMa
>>>>>
>>>>> On Sun, Apr 14, 2019, 7:56 AM <[email protected]> wrote:
>>>>>
>>>>>>
>>>>>> https://play.golang.org/p/6aQYNjojyBD
>>>>>>
>>>>>> I'm clearly missing something about the way sync.Mutex and
>>>>>> atomic.Value work in Go.
>>>>>>
>>>>>> I'm attempting to write a pair of concurrency safe methods, load()
>>>>>> and update(), for accessing a struct. The struct is stored as an
>>>>>> atomic.Value and accessed with atomic.Load and atomic.Value. I'm also
>>>>>> wrapping the accesses within a mutex Lock/Unlock. That's probably
>>>>>> unneeded
>>>>>> for my load() method but I added it trying to figure out why it's not
>>>>>> returning updated info. In my minimal example below (also in the Go
>>>>>> Playground link above) the output of main() should be:
>>>>>>
>>>>>> s={65535}
>>>>>> v={65535}
>>>>>>
>>>>>> but I get
>>>>>>
>>>>>> s={65535}
>>>>>> v={0}
>>>>>>
>>>>>> indicating that the updated value is not available after the call to
>>>>>> update().
>>>>>>
>>>>>> The only thing I'm doing that's a little different from the examples
>>>>>> in the doc for sync/atomic is passing a function that takes a pointer to
>>>>>> a
>>>>>> struct instance to my update function. I do that to make it easy to
>>>>>> write
>>>>>> code that updates just a few items in the state struct (which in my real
>>>>>> application has many members instead of just one as shown here.)
>>>>>>
>>>>>> Apologies for wasting the group's time if I've overlooked a
>>>>>> brain-dead error, but I've been fooling with this for several hours now
>>>>>> and
>>>>>> can't see why it shouldn't be working,
>>>>>>
>>>>>>
>>>>>> package main
>>>>>>
>>>>>> import (
>>>>>> "fmt"
>>>>>> "sync"
>>>>>> "sync/atomic"
>>>>>> )
>>>>>>
>>>>>> type state struct {
>>>>>> R4000 uint16
>>>>>> }
>>>>>> type guardedState struct {
>>>>>> v atomic.Value
>>>>>> }
>>>>>>
>>>>>> var stateMutex = sync.Mutex{}
>>>>>> var gState guardedState
>>>>>>
>>>>>> func init() {
>>>>>> gState.v.Store(state{})
>>>>>> }
>>>>>>
>>>>>> func (g guardedState) load() state {
>>>>>> stateMutex.Lock()
>>>>>> defer stateMutex.Unlock()
>>>>>> s := gState.v.Load()
>>>>>> return s.(state)
>>>>>> }
>>>>>>
>>>>>> func (g guardedState) update(f func(*state)) {
>>>>>> stateMutex.Lock()
>>>>>> defer stateMutex.Unlock()
>>>>>> s := g.v.Load().(state)
>>>>>> f(&s)
>>>>>> g.v.Store(s)
>>>>>> }
>>>>>>
>>>>>> func main() {
>>>>>> f := func(s *state) {
>>>>>> s.R4000 = 65535
>>>>>> fmt.Printf("s=%v\n", *s)
>>>>>> }
>>>>>> gState.update(f)
>>>>>> v := gState.load()
>>>>>> fmt.Printf("v=%v\n", v)
>>>>>> }
>>>>>>
>>>>>> --
>>>>>> 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 [email protected].
>>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>>>
>>>>> --
>>>> 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 [email protected].
>>>> For more options, visit https://groups.google.com/d/optout.
>>>>
>>> --
>> 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 [email protected] <javascript:>.
>> For more options, visit https://groups.google.com/d/optout.
>>
>
--
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 [email protected].
For more options, visit https://groups.google.com/d/optout.
