mohae / deepcopy

Deep copy things
MIT License
569 stars 122 forks source link

Custom copy #2

Closed lgylgy closed 8 years ago

lgylgy commented 8 years ago

hi,

I would like to use you package. However some structs in my application contain (exported) time.Time fields.

Would you know how to have those fields copied by your library? More generally, how to integrate a custom copy?

mohae commented 8 years ago

Structs are copied. time.Time is a struct. Just call deepcopy.Copy(). Note that an interface{} is returned so you'll need to assert the copy to its underlying type.

There isn't a way to integrate a custom copy. This should copy anything that is exported, though I'm not sure about funcs and channels as I didn't test those.

lgylgy commented 8 years ago

Yes, but time.Time has unexported fields:

func (s *TestSuite) Test(c *C) {
    now := time.Now()
    copy, ok := deepcopy.Copy(now).(time.Time)
    c.Assert(ok, Equals, true)
    c.Assert(now, DeepEquals, copy)
}
... obtained time.Time = time.Time{sec:63611187148, nsec:318097500, loc:(*time.Location)(0x11d2da0)} ("2016-10-04 16:12:28.3180975 +0200 CEST")
... expected time.Time = time.Time{sec:0, nsec:0, loc:(*time.Location)(nil)} ("0001-01-01 00:00:00 +0000 UTC")

For more context, in my application, i use this to copy a struct:

func DeepCopy(dst, src interface{}) interface{} {
    buffer := new(bytes.Buffer)
    err := gob.NewEncoder(buffer).Encode(src)
    if err != nil {
        panic(err)
    }
    err = gob.NewDecoder(buffer).Decode(dst)
    if err != nil {
        panic(err)
    }
    return dst
}

It's really slow and drastically affects the application performance. But it works for all types. I'm trying to think if i can add a hook to copy a specific type.

mohae commented 8 years ago

Unexported data cannot be copied without using unsafe; this is a limitation of the language. Fields are unexported for a reason. If you try to set an unexported field via package reflect you willl end up with a panic. To confirm you can elide this block of code: https://github.com/mohae/deepcopy/blob/master/deepcopy.go#L72:L74 and see what happens.

You can run the code in this gist: https://gist.github.com/mohae/ab31ae7d2c98c2cbf91f20f7266bb29b and see that deepcopy does what is expected: copies all exported fields in a struct.

If you need to copy unexported fields you're going to have to go the unsafe route.

mohae commented 8 years ago

In thinking about it a little more, I think, in regards to time.TIme, you'd have to set the unexported values you care about on the copy by doing the appropriate method calls on the original time.Time value to get the values and setting them on the copy with the appropriate methods.

Maybe this is something that could be handled by deepcopy for time.Time. I could see that being useful but it also could be going down the rabbit hole of special casing various structs in that way instead of just copying the exported fields.

I'm leaning towards that being something that something other than deepcopy should do instead of going down the rabbit hole of special casing behavior for various structs.

You could create a wrapper function that calls deepcopy.Copy() and then handles any copying of unexported values for the structs you care about.

mohae commented 8 years ago

Sorry for yet another post, but I just noticed the Gobs part.

In regards to unexported fields, Gob has the same limitation:

Functions and channels will not be sent in a gob. Attempting to encode such a value at the top level will fail. A struct field of chan or func type is treated exactly like an unexported field and is ignored.

I don't know what your overall use-case is; but there are faster serialization options than Gob. Also, depending on the use-case, re-using the Encoder and Decoder might help.

Also, I haven't benchmarked deepcopy; perhaps that's something I should do.

lgylgy commented 8 years ago

Thank you for your response. I think I'm doing a wrapper. I will get back to you if I have something interesting to show.

mohae commented 8 years ago

OK, thanks.

I'm going to assume it's ok to close this. Re-open if necessary.

Good luck!