Skip to content
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.

Sign up for GitHub

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

Drop Options argument from user-declared methods and functions #163

Merged
merged 1 commit into from
Feb 13, 2025
Merged

Drop Options argument from user-declared methods and functions #163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 15 additions & 45 deletions arshal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,35 +33,6 @@ var (
// export exposes internal functionality of the "jsontext" package.
var export = jsontext.Internal.Export(&internal.AllowInternalUse)

// mayReuseOpt reuses coderOpts if joining opts with the coderOpts
// would produce the equivalent set of options.
func mayReuseOpt(coderOpts *jsonopts.Struct, opts []Options) *jsonopts.Struct {
switch len(opts) {
// In the common case, the caller plumbs down options from the caller's caller,
// which is usually the [jsonopts.Struct] constructed by the top-level arshal call.
case 1:
o, _ := opts[0].(*jsonopts.Struct)
if o == coderOpts {
return coderOpts
}
// If the caller provides no options, then just reuse the coder's options,
// which may contain both marshaling/unmarshaling and encoding/decoding flags.
case 0:
return coderOpts
}
return nil
}

var structOptionsPool = &sync.Pool{New: func() any { return new(jsonopts.Struct) }}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay! We get to delete one of the pools.


func getStructOptions() *jsonopts.Struct {
return structOptionsPool.Get().(*jsonopts.Struct)
}
func putStructOptions(o *jsonopts.Struct) {
*o = jsonopts.Struct{}
structOptionsPool.Put(o)
}

// Marshal serializes a Go value as a []byte according to the provided
// marshal and encode options (while ignoring unmarshal or decode options).
// It does not terminate the output with a newline.
Expand Down Expand Up @@ -231,15 +202,13 @@ func MarshalWrite(out io.Writer, in any, opts ...Options) (err error) {
// See [Marshal] for details about the conversion of a Go value into JSON.
func MarshalEncode(out *jsontext.Encoder, in any, opts ...Options) (err error) {
xe := export.Encoder(out)
mo := mayReuseOpt(&xe.Struct, opts)
if mo == nil {
mo = getStructOptions()
defer putStructOptions(mo)
*mo = xe.Struct // initialize with encoder options before joining
mo.JoinWithoutCoderOptions(opts...)
if len(opts) > 0 {
optsOriginal := xe.Struct
defer func() { xe.Struct = optsOriginal }()
xe.Struct.JoinWithoutCoderOptions(opts...)
}
err = marshalEncode(out, in, mo)
if err != nil && mo.Flags.Get(jsonflags.ReportErrorsWithLegacySemantics) {
err = marshalEncode(out, in, &xe.Struct)
if err != nil && xe.Flags.Get(jsonflags.ReportErrorsWithLegacySemantics) {
return internal.TransformMarshalError(in, err)
}
return err
Expand Down Expand Up @@ -480,15 +449,13 @@ func unmarshalFull(in *jsontext.Decoder, out any, uo *jsonopts.Struct) error {
// See [Unmarshal] for details about the conversion of JSON into a Go value.
func UnmarshalDecode(in *jsontext.Decoder, out any, opts ...Options) (err error) {
xd := export.Decoder(in)
uo := mayReuseOpt(&xd.Struct, opts)
if uo == nil {
uo = getStructOptions()
defer putStructOptions(uo)
*uo = xd.Struct // initialize with decoder options before joining
uo.JoinWithoutCoderOptions(opts...)
if len(opts) > 0 {
optsOriginal := xd.Struct
defer func() { xd.Struct = optsOriginal }()
xd.Struct.JoinWithoutCoderOptions(opts...)
}
err = unmarshalDecode(in, out, uo)
if err != nil && uo.Flags.Get(jsonflags.ReportErrorsWithLegacySemantics) {
err = unmarshalDecode(in, out, &xd.Struct)
if err != nil && xd.Flags.Get(jsonflags.ReportErrorsWithLegacySemantics) {
return internal.TransformUnmarshalError(out, err)
}
return err
Expand Down Expand Up @@ -544,6 +511,9 @@ func newAddressableValue(t reflect.Type) addressableValue {
return addressableValue{reflect.New(t).Elem(), true}
}

// TODO: Remove *jsonopts.Struct argument from [marshaler] and [unmarshaler].
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this will speed up the performance of marshaling and unmarshaling since each recursive call doesn't need to keep passing the same *Struct pointer on the stack.

// This can be directly accessed on the encoder or decoder.

// All marshal and unmarshal behavior is implemented using these signatures.
// The *jsonopts.Struct argument is guaranteed to identical to or at least
// a strict super-set of the options in Encoder.Struct or Decoder.Struct.
Expand Down
12 changes: 6 additions & 6 deletions arshal_funcs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,9 @@ func MarshalFunc[T any](fn func(T) ([]byte, error)) *Marshalers {
// on the provided encoder. It may return [SkipFunc] such that marshaling can
// move on to the next marshal function. However, no mutable method calls may
// be called on the encoder if [SkipFunc] is returned.
// The pointer to [jsontext.Encoder], the value of T, and the [Options] value
// The pointer to [jsontext.Encoder] and the value of T
// must not be retained outside the function call.
func MarshalToFunc[T any](fn func(*jsontext.Encoder, T, Options) error) *Marshalers {
func MarshalToFunc[T any](fn func(*jsontext.Encoder, T) error) *Marshalers {
t := reflect.TypeFor[T]()
assertCastableTo(t, true)
typFnc := typedMarshaler{
Expand All @@ -220,7 +220,7 @@ func MarshalToFunc[T any](fn func(*jsontext.Encoder, T, Options) error) *Marshal
xe := export.Encoder(enc)
prevDepth, prevLength := xe.Tokens.DepthLength()
xe.Flags.Set(jsonflags.WithinArshalCall | 1)
err := fn(enc, va.castTo(t).Interface().(T), mo)
err := fn(enc, va.castTo(t).Interface().(T))
xe.Flags.Set(jsonflags.WithinArshalCall | 0)
currDepth, currLength := xe.Tokens.DepthLength()
if err == nil && (prevDepth != currDepth || prevLength+1 != currLength) {
Expand Down Expand Up @@ -291,9 +291,9 @@ func UnmarshalFunc[T any](fn func([]byte, T) error) *Unmarshalers {
// on the provided decoder. It may return [SkipFunc] such that unmarshaling can
// move on to the next unmarshal function. However, no mutable method calls may
// be called on the decoder if [SkipFunc] is returned.
// The pointer to [jsontext.Decoder], the value of T, and [Options] value
// The pointer to [jsontext.Decoder] and the value of T
// must not be retained outside the function call.
func UnmarshalFromFunc[T any](fn func(*jsontext.Decoder, T, Options) error) *Unmarshalers {
func UnmarshalFromFunc[T any](fn func(*jsontext.Decoder, T) error) *Unmarshalers {
t := reflect.TypeFor[T]()
assertCastableTo(t, false)
typFnc := typedUnmarshaler{
Expand All @@ -302,7 +302,7 @@ func UnmarshalFromFunc[T any](fn func(*jsontext.Decoder, T, Options) error) *Unm
xd := export.Decoder(dec)
prevDepth, prevLength := xd.Tokens.DepthLength()
xd.Flags.Set(jsonflags.WithinArshalCall | 1)
err := fn(dec, va.castTo(t).Interface().(T), uo)
err := fn(dec, va.castTo(t).Interface().(T))
xd.Flags.Set(jsonflags.WithinArshalCall | 0)
currDepth, currLength := xd.Tokens.DepthLength()
if err == nil && (prevDepth != currDepth || prevLength+1 != currLength) {
Expand Down
13 changes: 6 additions & 7 deletions arshal_methods.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ type Marshaler interface {
// should aim to have equivalent behavior for the default marshal options.
//
// The implementation must write only one JSON value to the Encoder and
// must not retain the pointer to [jsontext.Encoder] or the [Options] value.
// must not retain the pointer to [jsontext.Encoder].
type MarshalerTo interface {
MarshalJSONTo(*jsontext.Encoder, Options) error
MarshalJSONTo(*jsontext.Encoder) error

// TODO: Should users call the MarshalEncode function or
// should/can they call this method directly? Does it matter?
Expand Down Expand Up @@ -85,10 +85,9 @@ type Unmarshaler interface {
// It is recommended that UnmarshalJSONFrom implement merge semantics when
// unmarshaling into a pre-populated value.
//
// Implementations must not retain the pointer to [jsontext.Decoder] or
// the [Options] value.
// Implementations must not retain the pointer to [jsontext.Decoder].
type UnmarshalerFrom interface {
UnmarshalJSONFrom(*jsontext.Decoder, Options) error
UnmarshalJSONFrom(*jsontext.Decoder) error

// TODO: Should users call the UnmarshalDecode function or
// should/can they call this method directly? Does it matter?
Expand Down Expand Up @@ -193,7 +192,7 @@ func makeMethodArshaler(fncs *arshaler, t reflect.Type) *arshaler {
xe := export.Encoder(enc)
prevDepth, prevLength := xe.Tokens.DepthLength()
xe.Flags.Set(jsonflags.WithinArshalCall | 1)
err := va.Addr().Interface().(MarshalerTo).MarshalJSONTo(enc, mo)
err := va.Addr().Interface().(MarshalerTo).MarshalJSONTo(enc)
xe.Flags.Set(jsonflags.WithinArshalCall | 0)
currDepth, currLength := xe.Tokens.DepthLength()
if (prevDepth != currDepth || prevLength+1 != currLength) && err == nil {
Expand Down Expand Up @@ -283,7 +282,7 @@ func makeMethodArshaler(fncs *arshaler, t reflect.Type) *arshaler {
xd := export.Decoder(dec)
prevDepth, prevLength := xd.Tokens.DepthLength()
xd.Flags.Set(jsonflags.WithinArshalCall | 1)
err := va.Addr().Interface().(UnmarshalerFrom).UnmarshalJSONFrom(dec, uo)
err := va.Addr().Interface().(UnmarshalerFrom).UnmarshalJSONFrom(dec)
xd.Flags.Set(jsonflags.WithinArshalCall | 0)
currDepth, currLength := xd.Tokens.DepthLength()
if (prevDepth != currDepth || prevLength+1 != currLength) && err == nil {
Expand Down
Loading
Loading