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

IPLD Amend #1

Closed
wants to merge 8 commits into from
Closed

IPLD Amend #1

wants to merge 8 commits into from

Conversation

smrz2001
Copy link
Owner

@smrz2001 smrz2001 commented Mar 13, 2022

This PR implements a possible solution for incrementally modifying IPLD nodes.

  • @warpfork's patch implementation does wholesale, history-less Node updates but needs further optimization.
  • Draft implementation to accumulate updates and internal state for map-type top-level "add" operation.
  • Draft implementation to provide a Node "lens" for copy-on-write during Encode.
  • Implement multi-level "add".
  • Implement map-type "remove" operations.
  • Pass TestSpecFixtures/removing-map-entry.
  • Pass TestSpecFixtures/replacing-map-entry.
  • Pass TestSpecFixtures/copy.
  • Pass TestSpecFixtures/move.
  • Create Amender interface.
  • Implement list-type "add" operation.
  • Pass TestSpecFixtures/inserting-into-a-list.
  • Pass TestSpecFixtures/test-and-conditional-modify.
  • Implement list-type "remove" operation.
  • Add more comments.
  • Add more benchmarking tests
  • pprof analysis
  • Add fixtures for corner cases
  • Investigate issues while testing patch implementation

@smrz2001 smrz2001 self-assigned this Mar 13, 2022
@smrz2001 smrz2001 changed the base branch from master to patch-feature March 13, 2022 17:48
@warpfork
Copy link

This is superwild and very interesting!!

As an outline it definitely seems plausible that this can work. So I'll say no more about that.

What's most interesting here is promoting the "patch" operations and structures...

... from where I drafted them in the traversal/patch package -- a leaf node, which is not necessary to use (and thus we can evolve for a while with a more "alpha"-like attitude, and even build alternatives to in parallel)...

... into the core datamodel package -- which is the bottom of all imports (you can't avoid having in your import graph, and anything there almost immediately becomes highly change-resistant as a result of being so loadbearing).

I'm a little nervous about that, to say the least. 😅

The feature overlap is pretty high, so it makes sense in that regard. But it's not 100% -- for example, I can't imagine what use the "test" operation would be here (which we have in turn largely just because it's specified in the JSON Patch RFC, and I was rolling with compatibility with that in mind when writing that patch package). I'm also not sure if a path to patch op target makes sense; just a pathsegment for where to update the value within the current node (no deeper) should suffice? And for a third, "move" and "copy" might be a little strange -- from a single node's perspective, when we're working with the in-memory handles to other nodes already, we don't really need those ops, do we?

More important is the change coupling and inertia we'd be stuck with. I can't really provide evidence that would be bad, but I don't know if I would want to take a bet that it would be fine, either.

I wonder if something very like this, but declaring new, unrelated types in the data model package, would be slightly more ideal. It would be more code, and some of it alarmingly similar looking, but it would decouple things more, and I suspect it might end up more refined in the end.

Comment on lines 204 to 205
type NodePrototypeSupportingAmend interface {
AmendingBuilder(base Node) NodeBuilder
AmendingBuilder(base Node) NodeAmendingBuilder

Choose a reason for hiding this comment

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

I've started to wonder if NodePrototype is the right place to attach this -- and suspect not.

(yes, I realize this is my own old draft interface, and you're shifting it to be slightly more correct in this patch. heh.)

Larger comment back in the issue over here: ipld#320 (comment)

@smrz2001
Copy link
Owner Author

More important is the change coupling and inertia we'd be stuck with. I can't really provide evidence that would be bad, but I don't know if I would want to take a bet that it would be fine, either.

I wonder if something very like this, but declaring new, unrelated types in the data model package, would be slightly more ideal. It would be more code, and some of it alarmingly similar looking, but it would decouple things more, and I suspect it might end up more refined in the end.

That makes sense. I'll try to play around with some options here. I didn't want to branch out too drastically, but more decoupling definitely seems like the better path.

@smrz2001
Copy link
Owner Author

The feature overlap is pretty high, so it makes sense in that regard. But it's not 100% -- for example, I can't imagine what use the "test" operation would be here (which we have in turn largely just because it's specified in the JSON Patch RFC, and I was rolling with compatibility with that in mind when writing that patch package). I'm also not sure if a path to patch op target makes sense; just a pathsegment for where to update the value within the current node (no deeper) should suffice? And for a third, "move" and "copy" might be a little strange -- from a single node's perspective, when we're working with the in-memory handles to other nodes already, we don't really need those ops, do we?

This is very interesting, thanks for thinking about this, @warpfork. I can see how patch operations don't fully overlap with datamodel.

I'm coming to the realization of patch and traversal being somewhat related abstractions - both are interested in Node relationships, traversal in looking at them, and patch in modifying them. So maybe inside traversal is the right place for patch lol...

I'll think about and play around with this some more soon.

@smrz2001 smrz2001 force-pushed the patch-feature-with-amend branch 2 times, most recently from 556c381 to 554144e Compare May 8, 2022 14:32
panic("misuse")
}
func (a *amenderNode) Length() int64 {
return a.base.Length() + int64(len(a.addOps)) - int64(len(a.remOps))
Copy link

Choose a reason for hiding this comment

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

I don't think this can be accurate as far as a plain node is concerned, since we don't know anything about overlap and conflicts between add and remove ops (e.g. add a field twice); but maybe it doesn't matter here. But what would be the purpose or reporting a Length() that has this base+add-remove calculation?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Coincidentally, I was just refactoring this :) I'm thinking of keeping a single map of mods. That way, the "effective" length calculation as well as the conflict resolution between adds/removes are simpler.

Copy link
Owner Author

Choose a reason for hiding this comment

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

A non-nil value in the mods map means that a node was added (or added/removed/re-added, and so on), while a value of nil means that it was removed.

Copy link
Owner Author

Choose a reason for hiding this comment

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

The currently pushed code definitely has bugs that I'm fixing (please don't hesitate to point them out). Correcting it to be more intelligent w.r.t. types of nodes.

Copy link
Owner Author

@smrz2001 smrz2001 May 31, 2022

Choose a reason for hiding this comment

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

@rvagg, I've iterated a couple of times and pushed another update with all (current) fixtures passing for map-type nodes.

It has a similar base+add-remove type calculation as before (with some adjustments) so that Encode sees an accurate "effective length" for recursive nodes.

This approach also allows me to iterate over the contents more intelligently in order to provide an accurate "effective view" (lens?) that incorporates all accumulated removals and additions.

@smrz2001
Copy link
Owner Author

If you look at this code anytime soon (no rush from my side), @rvagg, I'm working on a better strategy for list-type nodes.

The current approach appears to be working reasonably well for map-type nodes but I need to use an underlying mods type that supports the types of operations possible on a list (perhaps using an arraylist or linkedlist instead).

I also have a question about what to do when an entirely new hierarchy gets created with a path that has a mixture of string and int segments. For example, for a node with these contents:

{
	"foo": ["bar", "baz"]
}

...does this operation:

{ "op": "add", "path": "/foo/1/0", "value": "qux" }

...result in this node (my preference):

{
	"foo": [
		"bar",
		["qux"],
		 "baz"
	]
}

...or this node?

{
	"foo": [
		"bar",
		{
			"0": "qux"
		},
		 "baz"
	]
}

Basically, I'm trying to determine the "right" type for a parent Node when constructing a nested hierarchy. The code currently blindly assumes a map-type parent, but that doesn't seem correct.

I thought of using the "type" of the segment, but it doesn't seem safe to assume that an int type segment implies a list-type Node since it's quite possible for a map key to be an integer string representation.

It also doesn't seem safe to use the underlying storage type of PathSegment to infer the Node type because:

  • The storage type is currently hidden (for good reason since it's an implementation detail).
  • A string storage type does not necessarily imply a map-type Node since it could just as easily be the string representation of a list-type Node index.

@RangerMauve
Copy link

Hello, we're thinking of merging the initial version of the Patch APIs in go-ipld-prime.

Would you like to sync up before we do so? ipld#350

If we don't hear back we'll merge in a couple of weeks.

@smrz2001
Copy link
Owner Author

Hello, we're thinking of merging the initial version of the Patch APIs in go-ipld-prime.

Would you like to sync up before we do so? ipld#350

If we don't hear back we'll merge in a couple of weeks.

Thanks for the note, @RangerMauve.

It's cool from my side to merge ipld#350 whenever you're ready. I've intentionally kept my current implementation separate and parallel to Eric's implementation. Can chat about this on the next IPLD team sync, if needed.

I should have my implementation in a testable/reviewable state in a week or so, and be ready to benchmark both implementations.

Assuming, of course, that the fundamental approach makes sense to @rvagg and @warpfork. It's still a WIP but my next iteration should be closer to the final state I have in mind.

@RangerMauve
Copy link

@smrz2001 Sweet. Lets talk on the next call and I'll merge the PR after that. 😁

@rvagg
Copy link

rvagg commented Jun 1, 2022

@smrz2001 there's some overlapping discussion on PathSegment type stuff over @ ipld/ipld#195 which is also looking at patch-like semantics but for URLs.

I think I'm mostly of the opinion that if you use paths, then you should just put up with what paths give you - which is basically something like: integers become indexes of lists and everything else is map keys. If you want to break out of this then you have to use an alternative mechanism. Which I think might be a case for either planning for a future extension to add more sophisticated specifiers than just plain paths, or building in an escaping mechanism to path semantics (although it could be argued that we're far too late for changing path semantics at all).

@smrz2001
Copy link
Owner Author

smrz2001 commented Jun 2, 2022

@smrz2001 there's some overlapping discussion on PathSegment type stuff over @ ipld/ipld#195 which is also looking at patch-like semantics but for URLs.

I think I'm mostly of the opinion that if you use paths, then you should just put up with what paths give you - which is basically something like: integers become indexes of lists and everything else is map keys. If you want to break out of this then you have to use an alternative mechanism. Which I think might be a case for either planning for a future extension to add more sophisticated specifiers than just plain paths, or building in an escaping mechanism to path semantics (although it could be argued that we're far too late for changing path semantics at all).

Thanks, @rvagg, this is very helpful!

@smrz2001 smrz2001 changed the title EXPERIMENTAL: IPLD Amend IPLD Amend Jun 7, 2022
@smrz2001 smrz2001 marked this pull request as ready for review June 7, 2022 15:21
@smrz2001
Copy link
Owner Author

smrz2001 commented Jun 7, 2022

@rvagg @RangerMauve, I've cleaned up the code, added comments, got all the tests to pass, and marked the PR ready for review.

I'll continue to tinker with the fixtures to cover more corner cases as well as add more comments.

I also feel like I could have used Golang more idiomatically but I'm not a Golang expert by any means so would also appreciate suggestions there.

I haven't implemented link traversal but have an idea of how and where it should be implemented.

What would you recommend for next steps? Benchmarking?

cc @warpfork @BigLep

@smrz2001 smrz2001 changed the base branch from patch-feature to master June 10, 2022 00:26
@smrz2001
Copy link
Owner Author

Sorry for the force pushes... I detest git submodules - I just took a clean branch from master, copied over my code, and pushed to the same PR.

@smrz2001
Copy link
Owner Author

smrz2001 commented Jun 11, 2022

@rvagg @RangerMauve, just pushed some crude benchmarking tests comparing the two implementations (calling them amend (mine) vs patch (from master) for convenience).

I see some patterns already based on the size of the base Node, the type of the Node, and the number of update operations, but obviously need to dig deeper. I'm also unable to get some of the tests to run with patch, but that could just be me using the API incorrectly.

My choice of how to implement the test was purely based on how quick/easy it was to write lol, but I might have unintentionally introduced some bias and will refine the tests some more.

I should probably also add a check at the end of each test to make sure the serialized results are correct/consistent.

Other than "remove" for lists, I was to able to run other operations ("add" and "replace") with both implementations using both recursive types. Here are some results for a N-sized map/list -> 1% / 10% / 100% update operations -> dagjson.Encode.

I only did 10 runs to start with to save some time iterating on the longer tests.

BenchmarkAmend_Map_Add/inputs:_{100_1}-16                   10                                   18226 B/op        159 allocs/op
BenchmarkAmend_Map_Add/inputs:_{100_10}-16                  10             52060 ns/op           26762 B/op        260 allocs/op
BenchmarkAmend_Map_Add/inputs:_{100_100}-16                 10            181140 ns/op           76978 B/op       1260 allocs/op
BenchmarkAmend_Map_Add/inputs:_{1000_10}-16                 10            314390 ns/op          210631 B/op       1613 allocs/op
BenchmarkAmend_Map_Add/inputs:_{1000_100}-16                10            362820 ns/op          250914 B/op       2611 allocs/op
BenchmarkAmend_Map_Add/inputs:_{1000_1000}-16               10           1482160 ns/op          791540 B/op      14336 allocs/op
BenchmarkAmend_Map_Add/inputs:_{10000_100}-16               10           3200010 ns/op         1891893 B/op      16115 allocs/op
BenchmarkAmend_Map_Add/inputs:_{10000_1000}-16              10           4916820 ns/op         2356992 B/op      27834 allocs/op
BenchmarkAmend_Map_Add/inputs:_{10000_10000}-16             10          15798460 ns/op         7207678 B/op     145021 allocs/op

BenchmarkPatch_Map_Add/inputs:_{100_1}-16                   10             99990 ns/op           33170 B/op        164 allocs/op
BenchmarkPatch_Map_Add/inputs:_{100_10}-16                  10            200160 ns/op          216511 B/op        364 allocs/op
BenchmarkPatch_Map_Add/inputs:_{100_100}-16                 10           2308320 ns/op         2572325 B/op       2333 allocs/op
BenchmarkPatch_Map_Add/inputs:_{1000_10}-16                 10           1584600 ns/op         2014735 B/op       1705 allocs/op
BenchmarkPatch_Map_Add/inputs:_{1000_100}-16                10          14111120 ns/op        17653588 B/op       3596 allocs/op
BenchmarkPatch_Map_Add/inputs:_{1000_1000}-16               10         205712340 ns/op       224642778 B/op      24701 allocs/op
BenchmarkPatch_Map_Add/inputs:_{10000_100}-16               10         133080910 ns/op       135146020 B/op      17103 allocs/op
BenchmarkPatch_Map_Add/inputs:_{10000_1000}-16              10        1389110410 ns/op       1368573700 B/op     51461 allocs/op
BenchmarkPatch_Map_Add/inputs:_{10000_10000}-16             10       22175545610 ns/op      20921887109 B/op    613898 allocs/op

BenchmarkAmend_List_Add/inputs:_{100_1}-16                  10                                   24301 B/op        241 allocs/op
BenchmarkAmend_List_Add/inputs:_{100_10}-16                 10            100010 ns/op           26605 B/op        304 allocs/op
BenchmarkAmend_List_Add/inputs:_{100_100}-16                10              5670 ns/op           61645 B/op        936 allocs/op
BenchmarkAmend_List_Add/inputs:_{1000_10}-16                10            200020 ns/op          230272 B/op       2377 allocs/op
BenchmarkAmend_List_Add/inputs:_{1000_100}-16               10            299940 ns/op          256224 B/op       3007 allocs/op
BenchmarkAmend_List_Add/inputs:_{1000_1000}-16              10            805470 ns/op          590208 B/op      10209 allocs/op
BenchmarkAmend_List_Add/inputs:_{10000_100}-16              10           2293590 ns/op         2244676 B/op      23710 allocs/op
BenchmarkAmend_List_Add/inputs:_{10000_1000}-16             10           5815670 ns/op         2726106 B/op      30911 allocs/op
BenchmarkAmend_List_Add/inputs:_{10000_10000}-16            10          47789820 ns/op         5790995 B/op     102913 allocs/op

BenchmarkPatch_List_Add/inputs:_{100_1}-16                  10            100020 ns/op           18789 B/op        142 allocs/op
BenchmarkPatch_List_Add/inputs:_{100_10}-16                 10             76810 ns/op           38733 B/op        250 allocs/op
BenchmarkPatch_List_Add/inputs:_{100_100}-16                10            212270 ns/op          319421 B/op       1331 allocs/op
BenchmarkPatch_List_Add/inputs:_{1000_10}-16                10            211800 ns/op          322560 B/op       1423 allocs/op
BenchmarkPatch_List_Add/inputs:_{1000_100}-16               10           1409890 ns/op         1993850 B/op       2503 allocs/op
BenchmarkPatch_List_Add/inputs:_{1000_1000}-16              10          19142810 ns/op        25922684 B/op      14206 allocs/op
BenchmarkPatch_List_Add/inputs:_{10000_100}-16              10          15042580 ns/op        17914061 B/op      14208 allocs/op
BenchmarkPatch_List_Add/inputs:_{10000_1000}-16             10         143552170 ns/op       174261019 B/op      25918 allocs/op
BenchmarkPatch_List_Add/inputs:_{10000_10000}-16            10        2196600260 ns/op       2446689104 B/op    143244 allocs/op

BenchmarkAmend_Map_Remove/inputs:_{100_1}-16                10                                   17874 B/op        152 allocs/op
BenchmarkAmend_Map_Remove/inputs:_{100_10}-16               10            100020 ns/op           18320 B/op        189 allocs/op
BenchmarkAmend_Map_Remove/inputs:_{100_100}-16              10             99960 ns/op           22703 B/op        551 allocs/op
BenchmarkAmend_Map_Remove/inputs:_{1000_10}-16              10            199920 ns/op          207111 B/op       1543 allocs/op
BenchmarkAmend_Map_Remove/inputs:_{1000_100}-16             10            299830 ns/op          216031 B/op       1912 allocs/op
BenchmarkAmend_Map_Remove/inputs:_{1000_1000}-16            10            599820 ns/op          294604 B/op       6424 allocs/op
BenchmarkAmend_Map_Remove/inputs:_{10000_100}-16            10           3600110 ns/op         1864284 B/op      15414 allocs/op
BenchmarkAmend_Map_Remove/inputs:_{10000_1000}-16           10           4199810 ns/op         2045996 B/op      19934 allocs/op
BenchmarkAmend_Map_Remove/inputs:_{10000_10000}-16          10           6597700 ns/op         2534543 B/op      65104 allocs/op

BenchmarkPatch_Map_Remove/inputs:_{100_1}-16                10             99900 ns/op           26156 B/op        154 allocs/op
BenchmarkPatch_Map_Remove/inputs:_{100_10}-16               10             99880 ns/op          102773 B/op        262 allocs/op
BenchmarkPatch_Map_Remove/inputs:_{100_100}-16              10            599890 ns/op          527784 B/op       1214 allocs/op
BenchmarkPatch_Map_Remove/inputs:_{1000_10}-16              10           1300150 ns/op         1355376 B/op       1605 allocs/op
BenchmarkPatch_Map_Remove/inputs:_{1000_100}-16             10          10380090 ns/op        11698952 B/op       2595 allocs/op
BenchmarkPatch_Map_Remove/inputs:_{1000_1000}-16            10          53222380 ns/op        53504904 B/op      13880 allocs/op
BenchmarkPatch_Map_Remove/inputs:_{10000_100}-16            10         121890980 ns/op        93760181 B/op      16099 allocs/op
BenchmarkPatch_Map_Remove/inputs:_{10000_1000}-16           10        1162040110 ns/op       908162999 B/op      26902 allocs/op
BenchmarkPatch_Map_Remove/inputs:_{10000_10000}-16          10        5893638170 ns/op       5023323998 B/op    217868 allocs/op

BenchmarkAmend_Map_Replace/inputs:_{100_1}-16               10            103950 ns/op           18034 B/op        155 allocs/op
BenchmarkAmend_Map_Replace/inputs:_{100_10}-16              10             99660 ns/op           20253 B/op        228 allocs/op
BenchmarkAmend_Map_Replace/inputs:_{100_100}-16             10            100080 ns/op           48069 B/op        917 allocs/op
BenchmarkAmend_Map_Replace/inputs:_{1000_10}-16             10            299870 ns/op          209055 B/op       1590 allocs/op
BenchmarkAmend_Map_Replace/inputs:_{1000_100}-16            10            405340 ns/op          235589 B/op       2398 allocs/op
BenchmarkAmend_Map_Replace/inputs:_{1000_1000}-16           10            912080 ns/op          471074 B/op      10956 allocs/op
BenchmarkAmend_Map_Replace/inputs:_{10000_100}-16           10           2922220 ns/op         1876106 B/op      15912 allocs/op
BenchmarkAmend_Map_Replace/inputs:_{10000_1000}-16          10           4390300 ns/op         2211032 B/op      24871 allocs/op
BenchmarkAmend_Map_Replace/inputs:_{10000_10000}-16         10          11723930 ns/op         4386428 B/op     111308 allocs/op

BenchmarkPatch_Map_Replace/inputs:_{100_1}-16               10                                   26300 B/op        157 allocs/op
BenchmarkPatch_Map_Replace/inputs:_{100_10}-16              10            199780 ns/op          107170 B/op        300 allocs/op
BenchmarkPatch_Map_Replace/inputs:_{100_100}-16             10           1217100 ns/op          917327 B/op       1722 allocs/op
BenchmarkPatch_Map_Replace/inputs:_{1000_10}-16             10           1406350 ns/op         1356876 B/op       1644 allocs/op
BenchmarkPatch_Map_Replace/inputs:_{1000_100}-16            10          11368690 ns/op        11714032 B/op       2987 allocs/op
BenchmarkPatch_Map_Replace/inputs:_{1000_1000}-16           10         112271600 ns/op        115287372 B/op     17311 allocs/op
BenchmarkPatch_Map_Replace/inputs:_{10000_100}-16           10         125314240 ns/op        94456392 B/op      16502 allocs/op
BenchmarkPatch_Map_Replace/inputs:_{10000_1000}-16          10        1239588670 ns/op        927938346 B/op     30920 allocs/op
BenchmarkPatch_Map_Replace/inputs:_{10000_10000}-16         10       13105201960 ns/op       9262856949 B/op   175152 allocs/op

BenchmarkAmend_List_Replace/inputs:_{100_1}-16              10                                   21885 B/op        240 allocs/op
BenchmarkAmend_List_Replace/inputs:_{100_10}-16             10            100030 ns/op           23181 B/op        303 allocs/op
BenchmarkAmend_List_Replace/inputs:_{100_100}-16            10                                   36141 B/op        933 allocs/op
BenchmarkAmend_List_Replace/inputs:_{1000_10}-16            10             99900 ns/op          229202 B/op       2386 allocs/op
BenchmarkAmend_List_Replace/inputs:_{1000_100}-16           10            199910 ns/op          242608 B/op       3098 allocs/op
BenchmarkAmend_List_Replace/inputs:_{1000_1000}-16          10            599930 ns/op          381332 B/op      11109 allocs/op
BenchmarkAmend_List_Replace/inputs:_{10000_100}-16          10           2099950 ns/op         2215556 B/op      23809 allocs/op
BenchmarkAmend_List_Replace/inputs:_{10000_1000}-16         10           2600190 ns/op         2358338 B/op      31900 allocs/op
BenchmarkAmend_List_Replace/inputs:_{10000_10000}-16        10           7323250 ns/op         3796512 B/op     112817 allocs/op

BenchmarkPatch_List_Replace/inputs:_{100_1}-16              10                                   16580 B/op        140 allocs/op
BenchmarkPatch_List_Replace/inputs:_{100_10}-16             10            101940 ns/op           35568 B/op        239 allocs/op
BenchmarkPatch_List_Replace/inputs:_{100_100}-16            10            200470 ns/op          224909 B/op       1230 allocs/op
BenchmarkPatch_List_Replace/inputs:_{1000_10}-16            10            199900 ns/op          321248 B/op       1421 allocs/op
BenchmarkPatch_List_Replace/inputs:_{1000_100}-16           10           1399810 ns/op         1822178 B/op       2492 allocs/op
BenchmarkPatch_List_Replace/inputs:_{1000_1000}-16          10          12712450 ns/op        16837754 B/op      14103 allocs/op
BenchmarkPatch_List_Replace/inputs:_{10000_100}-16          10          14976720 ns/op        17884236 B/op      14206 allocs/op
BenchmarkPatch_List_Replace/inputs:_{10000_1000}-16         10         138821510 ns/op       165614237 B/op      25911 allocs/op
BenchmarkPatch_List_Replace/inputs:_{10000_10000}-16        10        1446597090 ns/op       1643059943 B/op    142986 allocs/op

@RangerMauve
Copy link

Would it make sense to replace the current patch implementation with this instead?

e.g. expose an API to apply a patch set using the schema, but to have the underlying stuff using these new APIs / expose the raw APIs for when folks don't want to use patch sets?

@smrz2001
Copy link
Owner Author

Hey @RangerMauve, I think it'd probably be good to wait till @warpfork is back and also has a chance to review the code and some of the design choices.

There are also a few items that are not yet implemented in this version of the code that are available in patch, e.g. budgets, link traversal, etc. (and perhaps others I've missed?)

I can take a stab at implementing those as well before considering the PR finalized so that we don't regress what's currently available.

@smrz2001
Copy link
Owner Author

Closing in favor of official IPLD PR.

@smrz2001 smrz2001 closed this Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants