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

perf(crypto): replace bignumber.js with native BigInt #3010

Merged
merged 7 commits into from Oct 3, 2019

Conversation

@supaiku0
Copy link
Contributor

commented Oct 2, 2019

Summary

Replaces bignumber.js with our own thin wrapper around the native BigInt type. Depends on some yet unreleased @arkecosystem/utils fixes. With native BigInts the memory usage of my node is about 10% less after bootstrap compared to without this PR.

Benchmarks before:

  new Block()
    ✓  fromData (0) x 7,887 ops/sec ±11.72% (84 runs sampled)
    ✓  fromData (150) x 57.06 ops/sec ±3.20% (62 runs sampled)
    ✓  fromData (150 Multipayments, 1MB) x 10.76 ops/sec ±1.59% (30 runs sampled)
    ✓  fromData (150 Multipayments, 150 Recipients each) x 2.62 ops/sec ±4.45% (11 runs sampled)
    ✓  fromData (150 Multipayments, 500 Recipients each) x 0.83 ops/sec ±0.74% (7 runs sampled)
    ✓  fromHex (0) x 9,721 ops/sec ±1.63% (90 runs sampled)
    ✓  fromHex (150) x 64.47 ops/sec ±1.14% (67 runs sampled)
  Block.serialize (0 transactions)
    ✓  core x 172,060 ops/sec ±1.03% (89 runs sampled)
  Block.serialize (150 transactions)
    ✓  core x 711 ops/sec ±1.42% (91 runs sampled)
  Block.deserialize (0 transactions)
    ✓  core x 31,361 ops/sec ±0.99% (89 runs sampled)
  Block.deserialize (150 transactions)
    ✓  core x 69.22 ops/sec ±1.12% (71 runs sampled)
  new Transaction (Type 0)
    ✓  fromData x 8,947 ops/sec ±0.95% (93 runs sampled)
    ✓  fromHex x 10,821 ops/sec ±0.78% (89 runs sampled)
    ✓  fromBytes x 10,700 ops/sec ±0.81% (94 runs sampled)
    ✓  fromBytesUnsafe x 132,489 ops/sec ±0.57% (90 runs sampled)
  Transaction.serialize (Type 0)
    ✓  core x 124,110 ops/sec ±1.19% (92 runs sampled)
  Transaction.deserialize (Type 0)
    ✓  core x 120,541 ops/sec ±0.87% (92 runs sampled)

After:

  new Block()
    ✓  fromData (0) x 11,483 ops/sec ±10.84% (91 runs sampled)
    ✓  fromData (150) x 61.69 ops/sec ±1.75% (65 runs sampled)
    ✓  fromData (150 Multipayments, 1MB) x 11.59 ops/sec ±0.88% (32 runs sampled)
    ✓  fromData (150 Multipayments, 150 Recipients each) x 2.84 ops/sec ±4.53% (12 runs sampled)
    ✓  fromData (150 Multipayments, 500 Recipients each) x 0.90 ops/sec ±0.40% (7 runs sampled)
    ✓  fromHex (0) x 12,402 ops/sec ±1.07% (89 runs sampled)
    ✓  fromHex (150) x 65.93 ops/sec ±1.97% (68 runs sampled)
  Block.serialize (0 transactions)
    ✓  core x 321,789 ops/sec ±1.46% (94 runs sampled)
  Block.serialize (150 transactions)
    ✓  core x 723 ops/sec ±1.31% (89 runs sampled)
  Block.deserialize (0 transactions)
    ✓  core x 59,453 ops/sec ±0.79% (92 runs sampled)
  Block.deserialize (150 transactions)
    ✓  core x 71.12 ops/sec ±1.14% (74 runs sampled)
  new Transaction (Type 0)
    ✓  fromData x 9,646 ops/sec ±0.72% (90 runs sampled)
    ✓  fromHex x 11,406 ops/sec ±0.59% (89 runs sampled)
    ✓  fromBytes x 11,316 ops/sec ±0.76% (92 runs sampled)
    ✓  fromBytesUnsafe x 159,295 ops/sec ±1.29% (87 runs sampled)
  Transaction.serialize (Type 0)
    ✓  core x 124,942 ops/sec ±1.24% (90 runs sampled)
  Transaction.deserialize (Type 0)
    ✓  core x 141,753 ops/sec ±1.07% (91 runs sampled)

Checklist

  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged
@supaiku0 supaiku0 changed the title perf(crypto): replace bignumber.js with native bigint perf(crypto): replace bignumber.js with native BigInt Oct 2, 2019
supaiku0 added 2 commits Oct 3, 2019
@codecov

This comment has been minimized.

Copy link

commented Oct 3, 2019

Codecov Report

Merging #3010 into develop will increase coverage by 0.01%.
The diff coverage is 91.93%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3010      +/-   ##
===========================================
+ Coverage    66.03%   66.05%   +0.01%     
===========================================
  Files          423      423              
  Lines        10120    10131      +11     
  Branches       538      539       +1     
===========================================
+ Hits          6683     6692       +9     
- Misses        3392     3394       +2     
  Partials        45       45
Impacted Files Coverage Δ
packages/core-transaction-pool/src/dynamic-fee.ts 100% <ø> (ø) ⬆️
packages/core/src/commands/network/generate.ts 0% <0%> (ø) ⬆️
packages/core-utils/src/delegate-calculator.ts 100% <100%> (ø) ⬆️
...ackages/crypto/src/transactions/types/htlc-lock.ts 100% <100%> (ø) ⬆️
packages/crypto/src/blocks/serializer.ts 94.59% <100%> (+0.15%) ⬆️
packages/crypto/src/transactions/serializer.ts 88.7% <100%> (+0.09%) ⬆️
packages/crypto/src/utils/bignum.ts 100% <100%> (ø) ⬆️
packages/core-webhooks/src/conditions.ts 94.73% <100%> (-5.27%) ⬇️
packages/crypto/src/blocks/block.ts 90% <100%> (+0.78%) ⬆️
packages/crypto/src/transactions/types/transfer.ts 100% <100%> (ø) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4953e48...abd47de. Read the comment docs.

@faustbrian faustbrian merged commit 7469ee9 into develop Oct 3, 2019
83 checks passed
83 checks passed
crypto (10.x)
Details
bridgechain-registration (10.x)
Details
unit (10.x)
Details
crypto (12.x)
Details
bridgechain-registration (12.x)
Details
unit (12.x)
Details
bridgechain-resignation (10.x)
Details
integration (10.x)
Details
bridgechain-resignation (12.x)
Details
integration (12.x)
Details
bridgechain-update (10.x)
Details
e2e (10.x)
Details
bridgechain-update (12.x)
Details
e2e (12.x)
Details
business-registration (10.x)
Details
business-registration (12.x)
Details
business-resignation (10.x)
Details
business-resignation (12.x)
Details
business-update (10.x)
Details
business-update (12.x)
Details
delegate-registration (10.x)
Details
delegate-registration (12.x)
Details
delegate-resignation (10.x)
Details
delegate-resignation (12.x)
Details
htlc-claim (10.x)
Details
htlc-claim (12.x)
Details
htlc-lock (10.x)
Details
htlc-lock (12.x)
Details
htlc-refund (10.x)
Details
htlc-refund (12.x)
Details
ipfs (10.x)
Details
ipfs (12.x)
Details
multi-payment (10.x)
Details
multi-payment (12.x)
Details
multi-signature-registration (10.x)
Details
multi-signature-registration (12.x)
Details
second-signature-registration (10.x)
Details
second-signature-registration (12.x)
Details
transfer (10.x)
Details
transfer (12.x)
Details
vote (10.x)
Details
vote (12.x)
Details
LGTM analysis: JavaScript No new or fixed alerts
Details
codeclimate Approved by Brian Faust.
Details
codecov/patch 91.93% of diff hit (target 66.03%)
Details
codecov/project 66.05% (+0.01%) compared to 4953e48
Details
security/snyk - __tests__/e2e/package.json (ArkEcosystem) No manifest changes detected
security/snyk - package.json (ArkEcosystem) No new, high severity issues
Details
security/snyk - packages/core-api/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-blockchain/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-container/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-database-postgres/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-database/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-elasticsearch/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-error-tracker-airbrake/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-error-tracker-bugsnag/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-error-tracker-raygun/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-error-tracker-rollbar/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-error-tracker-sentry/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-event-emitter/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-exchange-json-rpc/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-explorer/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-forger/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-http-utils/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-interfaces/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-jest-matchers/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-logger-pino/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-logger-signale/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-logger-winston/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-logger/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-new-relic/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-p2p/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-snapshots/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-state/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-tester-cli/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-transaction-pool/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-transactions/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-utils/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-vote-report/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-wallet-api/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core-webhooks/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/core/package.json (ArkEcosystem) No manifest changes detected
security/snyk - packages/crypto/package.json (ArkEcosystem) No new, high severity issues
Details
@ArkEcosystemBot ArkEcosystemBot deleted the perf/crypto/native-bigint branch Oct 3, 2019
vasild added a commit that referenced this pull request Oct 4, 2019
…ts-nonce

* ArkEcosystem/core/develop:
  refactor: remove `vendorFieldHex` (#3014)
  ci: run push events on master and develop, rest on PRs
  perf(crypto): memoize base58 de/encoding (#3015)
  refactor(core-transactions): bootstrap transactions in batches (#2997)
  perf(crypto): replace bignumber.js with native BigInt (#3010)
  refactor: add `round.missed` event (#3011)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.