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.

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

feat!: use generics instead of cbg.Deferred #122

Closed
wants to merge 9 commits into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Aug 2, 2024

Experimenting with generics to do away with the deferred value decoding step.

The generic must conform to [T HamtValue[T]] where:

type HamtValue[T any] interface {
	Equal(T) bool
	MarshalCBOR(io.Writer) error
	UnmarshalCBOR(io.Reader) error
}
  • * pointers are kind of awkward in this, tho I'm not sure I can do much better without heavier changes (e.g. there's situations where you want to pass nil to indicate deletion)
  • The Equal is a little annoying, but necessary to know whether we're mutating in certain situations
  • FindRaw is gone, doesn't make sense anymore I think.
  • Find has a current use-case where you pass nil as the decoder and that means "I just want to know if it's there"; for certain types T it'll be more expensive to decode the full node with T elements than it was to do the cbg.Deferred cbor token skip job. Maybe.
  • Needs some benchmarking in cases like the lotus actor state migration where I'm suspecting this might be of benefit.
  • Maybe this could be /v4 but we still retain the /v3 variant for some cases.

@rvagg
Copy link
Member Author

rvagg commented Aug 5, 2024

Experimentation with this going on over at filecoin-project/go-state-types#298 against a >3M entry HAMT.

@rvagg rvagg marked this pull request as ready for review August 5, 2024 02:20
@rvagg
Copy link
Member Author

rvagg commented Aug 5, 2024

Rebased on #121, which has the restored cbor-gen code in it, so this now depends on whyrusleeping/cbor-gen#103 for cbor-gen generics support.

@rvagg
Copy link
Member Author

rvagg commented Oct 8, 2024

Bailing on this for now, as per whyrusleeping/cbor-gen#103 (comment)

@rvagg rvagg closed this Oct 8, 2024
@rvagg rvagg deleted the rvagg/generics branch October 8, 2024 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging this pull request may close these issues.

2 participants