On Monday, April 15, 2019 at 8:16:53 AM UTC-5, michae...@gmail.com wrote:
> 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 <michae...@gmail.com> 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 <michae...@gmail.com> 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  <michae...@gmail.com> 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 golan...@googlegroups.com.
> 
> 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 golan...@googlegroups.com.
> 
> 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 golan...@googlegroups.com.
> 
> For more options, visit https://groups.google.com/d/optout.

Thanks

-- 
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.

Reply via email to