-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Marshaler and Unmarshaler interfaces #17
Conversation
Add Marshaler and Unmarshaler interfaces, which allow for custom encoding/decoding working similarly to encoding/json Marshaler and Unmarshaler: ``` type Marshaler interface { MarshalXDR(e *Encoder) (int, error) } type Unmarshaler interface { UnmarshalXDR(e *Decoder) (int, error) } ``` The Decoder and Encoder automatically call the relevant methods if found. Both implementation by value and by pointer are supported. A new error (`ErrCustomUnaddressableByPointer`) will be returned if a by-pointer implementation is found but the value passed is unaddressable (not allowing to call the method by pointer).
93311b5
to
535624d
Compare
@@ -770,6 +776,37 @@ func (d *Decoder) decodeInterface(v reflect.Value) (int, error) { | |||
return d.decode(ve, 0) | |||
} | |||
|
|||
func getUnmarshaler(ve reflect.Value) Unmarshaler { | |||
// Check the type first to avoid an unnecessary allocation when casting to interface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is type switch also allocating? I wonder if can avoid using reflect
(which usually is slow). See: https://stackoverflow.com/a/28027945/9812359 maybe we can add a small benchmark to check it out. I think, it can simplify the code a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the tests I performed, I don't think it allocates.
@@ -1013,6 +1063,13 @@ func (d *Decoder) decodePtr(v reflect.Value) (int, error) { | |||
return n, err | |||
} | |||
|
|||
// We intentionally check for Marshaler before deferencing the pointer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// We intentionally check for Marshaler before deferencing the pointer. | |
// We intentionally check for Unmarshaler before deferencing the pointer. |
@@ -1013,6 +1063,13 @@ func (d *Decoder) decodePtr(v reflect.Value) (int, error) { | |||
return n, err | |||
} | |||
|
|||
// We intentionally check for Marshaler before deferencing the pointer. | |||
// This is because, for unaddressable values, you cannot recover the | |||
// pointer later on, which would make impossible to invoke by-address marshalers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// pointer later on, which would make impossible to invoke by-address marshalers. | |
// pointer later on, which would make impossible to invoke by-address unmarshalers. |
I think I am going to close this, because after updating xdrgen to generate Using by-value invocations, can (and does) cause allocations, which impacts performance quite negatively. See stellar/xdrgen#67 (comment) for more details. |
Add Marshaler and Unmarshaler interfaces, which allow for custom encoding/decoding
working similarly to
encoding/json
Marshaler and Unmarshaler:The Decoder and Encoder automatically call the relevant methods if found.
Both implementation by value and by pointer are supported.
A new error (
ErrCustomUnaddressableByPointer
) will be returned ifa by-pointer implementation is found but the value passed is unaddressable
(not allowing to call the method by pointer).
Note that
Marshaler
is intended to override theEncodeTo
methods introduced at stellar/go#3957 . I will later update xdrgen to generateMarshalXDR
implementations instead.