Add digestObjects method #58
Conversation
|
This pull request introduces 14 alerts when merging e4814b7 into 2d97993 - view on LGTM.com new alerts:
|
|
the benchmark shows that |
|
yeah! Approach looks very promising. @tuyennhv Can you adds benchmarks to convert to and from HashObjects? Either here or in ssz. |
@dapplion benchmark tests added for |
|
Looks great! @tuyennhv can you setup the benchmarks to run in CI? @wemeetagain Could you add credentials to this repo too? 🙏 |
@wemeetagain the benchmark workflow is ready, got |
Performance Report✔️ no performance regression detected Full benchmark results
|
|
@wemeetagain do I need to add this project to the global secrets? |
test/bench.js
Outdated
| const Benchmark = require('benchmark'); | ||
|
|
||
| const sha256 = require('../lib'); | ||
|
|
||
| const suite = new Benchmark.Suite; | ||
|
|
||
| const randomBuffer = (length) => Buffer.from(Array.from({length}, () => Math.round(Math.random() * 255))); | ||
|
|
||
| suite | ||
| .add('input length 32', () => sha256.default.digest(randomBuffer(32))) | ||
| .add('input length 64', () => sha256.default.digest(randomBuffer(64))) | ||
| .add('input digest-64', () => sha256.default.digest64(randomBuffer(64))) | ||
| .add('input length 128', () => sha256.default.digest(randomBuffer(128))) | ||
| .add('input length 256', () => sha256.default.digest(randomBuffer(256))) | ||
| .add('input length 512', () => sha256.default.digest(randomBuffer(512))) | ||
| .on('cycle', function (event) { | ||
| console.log(String(event.target)); | ||
| }).run(); |
There was a problem hiding this comment.
shouldn't this file stay as this provides perspective of how digest fn is scaling w.r.t. length while newer benchmark.test.js purpose seems more geared towards comparision.
There was a problem hiding this comment.
@g11tech I'll port it to the new benchmark framework
|
@GregTheGreek I already added the secrets locally |
twoeths
force-pushed
the
tuyen/digestObjects
branch
from
98c09f8 to
ebef2cf
Compare
Motivation
persistent-merkle-treenodes takes a lot of memory, v8 developers advised not to go with thatDescription
digestObjectsmethod to accept 2 objects ofh0toh7number, each number is equal to 4 bytes, that makes it equivalent to 1 Uint8Array(32)persistent-merkle-treenodes so that it'll share the same Objectmapmemory, storing these properties take roughly extra 32 bytes per node only, see Cache roots as 8 inline numbers persistent-merkle-tree#52Performance
This is from the performance test
AlthoughdigestObjectsis not as good as the newdigest64, it's already better than the currentdigest64of masterEdit:
digestObjectshas the same performance to the newdigest64thanks to unrolling for loops.Later on we can pass 8
u32numbers to wasm and do the bit shifting work there, that'd make it even better I think.Memory saving
Created a test in ssz with 250k validators.
heapUsedstarts with 870MB, callinghashTreeRootmakes it 1.33GB (with additional 460MB for Uint8Array roots)hashTreeRoot. This is becauseh0, ...h7already starts with 0. The produced root hex string is the same.h0toh7.