net-ipfs-core
net-ipfs-core copied to clipboard
richardschneider
Span<T>
Let's add Span<T>/Memory<T> into the library. This is an enhancement, current methods stay the same, we just add new method overloads.
A proof-of-concept should be the MultiHash class.
@richardschneider Sure, and FYI, here's an interesting discussion around introducing Span<T> to protobuf lib: https://github.com/protocolbuffers/protobuf/issues/3431
Just for the record, here's a good article https://msdn.microsoft.com/en-us/magazine/mt814808.aspx?f=255&MSPPError=-2147217396
@hanabi1224 I noticed you submitted a PR to multiformats. This library is not used by net-ipfs-core. We have our own multiformats in this project.
@richardschneider From what I understand, Span is about to allocate stack memory as appose to heap to reduce GC overhead thus get perf boost. It benefits scenaios that do heavy calculation / byte array munapulation the most(e.g. base58 encoding / decoding), I notice net-ipfs-core is using SimpleBase which does not have span support, then it's hard to get the benefit by using Span. FYI I have submitted PR to multibase to impelement Span version of base58 and benchmark shows good perf gain. If that is merged, maybe you can consider switching to that one (SimpleBase is external dependency anyway). Does that make sense?
@richardschneider Also I notice that the current Multihash decoding logic leveages on CodedInputStream in protobuf which unfortunately does not support Span api, Regarding the most import function is GetVarInt, I found a good replacement in lib BinaryEncoding, I have helped the author release a version that supports Span api yesterday which should be much more performant than protobuf ReadInt32. To benefit from Span api, you may want to switch to that lib. (But I would not expect good perf gain here, it's mostly just a read function than doing calculation)