-
Notifications
You must be signed in to change notification settings - Fork 163
Remove ipld
module in favour of libipld
dependancy
#494
Conversation
51ce9b7
to
d658ecf
Compare
At one point I remember we adopted the current cut down cbor implementation for if I can remember correctly the ipld imposing the base traits upon the whole system, like how to store blocks. Has that changed? I remembered of a similar issue; Cid/Multihash moving to using large allocationless storage instead of small Bytes allocations. Has there been any development on that front? I had a mental TODO on to investigate how this would change things for example for the unixfs, investigate other Cid implementations. While using prost in unixfs would make sense in my mind instead of quick-protobuf, I don't think it is a good idea to build such functionality on top of ipld object model, which I assume this PR is planning to. |
This would indeed be a problem. The current versions of |
This solution would still use even more space than the current |
Yes. I also though if it would perhaps make sense to have CID implementation that is not optimized for no_std environments and would just use heap allocations and hence be way simpler than the current |
@koivunej, Similarly, while IMO it might be more maintainable to refactor W.r.t. CIDs and Multihashes, I'm not well versed in exactly why things are the way they are implementation-wise, but the benchmark has improvements of ~20% for 1000 elements and ~50% for 10000 elements. Is this indicative of improvements in more recent versions of these crates? |
With the
Unixfs is just protobuf reading and writing, you don't need a central ipld object model.
I'm not following what benchmark you are talking about. Could you link to it? |
That approach makes a lot of sense IMO, I was just thinking about using libipld to handle the protobuf parts to reduce dependancies and to make some things clearer (e.g. I was referring to |
I don't think dhat-rs existed back then when the ingest-tar was created, but I'd be surprised if you don't see any difference in memory usage. (Cut a very long answer on benchmarking difficulties but maybe I should start up a blog once more :)) For the Ipld specific operations much more efficient ones could be implemented without the Ipld object model, as in, guiding the deserialization on the interesting path or links for example for references, similar to Users should be able to integrate with libipld without it's inclusion by implementing the Store (if that is even necessary). If there's something preventing that, we should fix it. Another aspect which makes me uneasy about including a such a heavy dependency is that we are already heavily dependent on the rust-libp2p, and upgrading it already takes a long while. If we'd add more such dependencies, which would sometimes conflict, of equally great importance, it would make everything a balancing act between PR'ing different repos and coordinating for releases. We ended up in adopting the dag-cbor and -json to ease this sitatuation. |
This PR replaces the IPLD module of
rust-ipfs
with that oflibipld
. This was proposed in issue #54, but the issue was closed (I believe upon the emergence ofipfs-embed
). This change would:rust-ipfs
to make use of the improved functionality found inlibipld
Current status: all changes required to build the project have been made. Some tests (specifically in
unixfs
) fail withProtobuf(UnexpectedEndOfBuffer)
while writing to a buffer, which is being investigated. There are probably more areas which can be improved by deduplicating some code (e.g.dag
andunixfs
can benefit from using somelibipld
functionality instead of re-implementing it, particularly relating to manual protobuf manipulation).Checklist
libipld
and does not re-implementBlock
Cid
,Multihash
,Multibase
and other IPLD related implementations now come fromlibipld
and are up to dateipld
module entirely removeddag
andunixfs
don't contain functionality which is duplicated inlibipld