search

interledgerjs / ilp-packet

Moved to monorepo in interledgerjs/interledgerjs
https://github.com/interledgerjs/interledgerjs
Other
6 stars 5 forks source link

perf: use new oer utils #48

Closed justmoon closed 5 years ago

justmoon commented 5 years ago

Updated to latest oer-utils with improved performance.

Did a quick benchmark of bignumber.js vs long. Long is ~33 times faster:

Long x 5,546 ops/sec ±2.75% (84 runs sampled)
Bignumber.js x 165 ops/sec ±1.94% (74 runs sampled)
Fastest is Long

This also allows us removing bignumber.js as a dependency of ilp-packet. Obviously, it' still an indirect dependency via oer-utils but it isn't used at all for (de)serializing ilp-packets. It might be worth making an oer-utils-light which doesn't depend on bignumber.js at all. I don't think anything in ILP actually needs support for >64 bit numbers.

Edit: Updated the benchmark to add deserialization. About 10x difference.

Long serialize x 8,195 ops/sec ±3.41% (81 runs sampled)
Bignumber.js serialize x 136 ops/sec ±9.00% (63 runs sampled)
Long deserialize x 7,317 ops/sec ±2.83% (81 runs sampled)
Bignumber.js deserialize x 708 ops/sec ±1.95% (83 runs sampled)
sharafian commented 5 years ago

Even for those totals we could theoretically wrap them in a class that uses long until it exceeds 64 bits and then switched to bignum

justmoon commented 5 years ago

I think it would be totally reasonable to cap our STREAM reference implementation at UInt64. For reference, that would introduce a limit of $18.4 billion at nanodollar resolution. So I wouldn't edit the spec to say that STREAM - the protocol - has a cap of UInt64 but we may want to add a note that some implementations may have a cap.

If you are actually going to send that kind of money over STREAM, I would hope you'd be using a more enterprise-y implementation than the JavaScript reference one anyway.

cc/ @sentientwaffle @emschwartz