Skip to content
This repository has been archived by the owner on Jan 20, 2020. It is now read-only.

Faster decimal library. #245

Merged
merged 1 commit into from
Jan 24, 2018
Merged

Faster decimal library. #245

merged 1 commit into from
Jan 24, 2018

Conversation

lisbakke
Copy link
Contributor

I was loading a lot of a data and noticed that the num library was slow. I gave decimal.js a shot and it's significantly faster.

@rmm5t
Copy link
Contributor

rmm5t commented Jan 23, 2018

I gave decimal.js a shot and it's significantly faster.

I'm in favor of dropping num (because it's a risk from the perspective of canonical popularity), but do you have any benchmark metrics to prove this claim?

Personally, I'd love to see a benchmark comparison between num, decimal.js, big.js, and bignumber.js.

If nothing else, big.js and bignumber.js win from a familiarity and popularity perspective (according to NPM downloads and github stars). Edit: Decided this is irrelevant considering the author is the same for these and decimal.js, but the statement holds as an argument against num.

@rmm5t
Copy link
Contributor

rmm5t commented Jan 23, 2018

This got me searching. Interestingly, while decimal.js was written to be "v2" of bigdecimal.js, the author of both claims that bigdecimal.js is potentially better for financial applications; whereas, decimal.js is potentially better for scientific applications:

https://github.com/MikeMcl/big.js/wiki

decimal.js was orginally intended to be version two of bignumber.js but I decided to make them separate libraries. The main difference between them is that with decimal.js precision is specified in terms of significant digits instead of decimal places, and all calculations are rounded to the precision (similar to Python's decimal module) rather than just those involving division.

This could be considered to make bignumber.js easier to use than decimal.js, and perhaps more suitable for financial applications, because the user doesn't need to worry about losing precision unless an operation involving division is used.

Conversely, decimal.js may be considered better for more scientifc applications as it can handle very small or large values more efficiently: for example, it does not have the limitation of bignumber.js that if adding a BigNumber value with a small exponent to one with a large exponent, bignumber.js will attempt to perform the operation to full precision which may involve the creation of a massive array, and consequently cause the operation to hang.

decimal.js also supports non-integer powers and adds the trigonometric functions and exp, ln, and log methods. These additions make decimal.js significantly larger than bignumber.js.

Though, the author also claims that decimal.js is faster than bignumer.js:

https://github.com/MikeMcl/decimal.js/#features

Faster, smaller, and perhaps easier to use than JavaScript versions of Java's BigDecimal

@lisbakke
Copy link
Contributor Author

I could provide metrics for decimal.js vs. num in the morning if you'd like. It's near 10x faster when loading the level 3 order book for the first time at https://github.com/coinbase/gdax-node/blob/master/lib/orderbook.js#L18

@lisbakke
Copy link
Contributor Author

lisbakke commented Jan 23, 2018

I just did 2 runs each to time orderbook.js line 18 -> 34 (loading new orderbook).

decimal.js 850ms & 805ms
num 12,200ms & 12,300ms

@rmm5t
Copy link
Contributor

rmm5t commented Jan 23, 2018

It's near 10x faster when loading the level 3 order book for the first time

Very nice! However, given that decimal.js only handles precision rounding (significant digits), but bignumber.js handles decimal place rounding, I think bignumber.js is a much better fit for a cryptocurrency/financial library.

Thoughts?

@lisbakke
Copy link
Contributor Author

Changed to BigNumber.js, performance looks good 👍

Copy link
Contributor

@rmm5t rmm5t left a comment

Choose a reason for hiding this comment

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

LGTM.

The only nit I have is that, by default, BigNumber is configured for 20 decimal places, but GDAX is limited to 8 decimal places. I think it would probably be safe to configure BigNumber with 10 decimal places (to account for rounding during potential mathematical operations -- even though none of those exist in the library just yet).

BigNumber.config({ DECIMAL_PLACES: 10 })

@nikulis
Copy link
Contributor

nikulis commented Jan 23, 2018

BigNumber is also what GDAX Trading Toolkit uses, so that consistency between Coinbase projects would be nice. (my 2¢)

@fb55
Copy link
Contributor

fb55 commented Jan 24, 2018

This looks great. I agree that we should probably switch to another library — BigNumber or Big.js would both be two great options.

@fb55 fb55 merged commit ab96d08 into coinbase:master Jan 24, 2018
@fb55
Copy link
Contributor

fb55 commented Jan 24, 2018

Missed the update — great addition, thanks a lot!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants