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

Implement Number64UintArray and tree_newTreeFromUint64Deltas #159

Merged
merged 7 commits into from
Aug 28, 2021

Conversation

twoeths
Copy link
Contributor

@twoeths twoeths commented Aug 19, 2021

Motivation

  1. Since persistent-merkle-tree internally track HashObject, we want to use it directly instead of having intermediate Uint8Array
  2. HashObject contains 8 32 bytes number which is suitable for serialize/deserialize uint
  3. Instead of having to serialize/deserialize to apply a delta, like balances[i] = balances[i] + delta we want a method to apply delta directly to an item in BasicArrayType
  4. New method to apply deltas and return a new tree, this is a convenient method for lodestar to process/apply rewards and penalties

Description

  • New Number64UintType for 64 bit uint, this is better than `NumberUintType({byteLength: 8}) - performance test proved that
  • Number64UintType works on HashObject instead of bytes, use it in Container and newly created Number64ListType
  • Number64ListType: new tree_applyUint64Delta method, this is for lodestar decreaseBalance increaseBalance scenario
  • Number64ListType: new tree_newTreeFromUint64Deltas, this is for lodestar processRewardsAndPenalties scenario

Closes #156

I applied this branch locally and got a better benchmark test result in lodestar (the result comes from different improvement, not only this PR)

# master new branch
beforeProcessEpoch 1.603897 s/op 697.9248 ms/op
processRewardsAndPenalties 704.3871 ms/op 138.6842 ms/op

Follow up work after this PR:

  • Use gindexbitstring whenever possible
  • Consider applying same strategy for BigIntUintType if bytelength = 8

@twoeths twoeths force-pushed the tuyen/Number64UintType branch from d152d46 to eb0734d Compare August 19, 2021 10:58
@twoeths twoeths marked this pull request as ready for review August 19, 2021 11:01
@twoeths twoeths force-pushed the tuyen/Number64UintType branch from c1d4cd3 to be27eae Compare August 19, 2021 12:08
@github-actions
Copy link

github-actions bot commented Aug 20, 2021

Performance Report

✔️ no performance regression detected

Full benchmark results
Benchmark suite Current: 38cfc35 Previous: 0d95532 Ratio
Array - for of 7.6310 us/op 11.332 us/op 0.67
Array - for(;;) 7.2230 us/op 10.650 us/op 0.68
basicListValue.readonlyValues() 221.30 ms/op 229.65 ms/op 0.96
basicListValue.readonlyValuesArray() 196.15 ms/op 195.51 ms/op 1.00
basicListValue.readonlyValues() + iterate all 213.83 ms/op 216.67 ms/op 0.99
basicListValue.readonlyValuesArray() + loop all 208.70 ms/op 201.66 ms/op 1.03
compositeListValue.readonlyValues() 409.91 ms/op 390.38 ms/op 1.05
compositeListValue.readonlyValuesArray() 385.03 ms/op 352.71 ms/op 1.09
compositeListValue.readonlyValues() + iterate all 317.27 ms/op 292.31 ms/op 1.09
compositeListValue.readonlyValuesArray() + loop all 387.87 ms/op 362.15 ms/op 1.07
NumberUintType - get balances list 716.86 ms/op
Number64UintType - get balances list 484.83 ms/op
NumberUintType - set balances list 1.1431 s/op
Number64UintType - set balances list 991.05 ms/op
Number64UintType - get and increase 10 then set 1.3697 s/op
Number64UintType - increase 10 using applyDelta 562.71 ms/op
tree_newTreeFromUint64Deltas 47.635 ms/op
unsafeUint8ArrayToTree 52.683 ms/op
Container {a,b,vec} - as struct x100000 69.125 us/op 61.997 us/op 1.11
Container {a,b,vec} - as tree x100000 111.17 ms/op 105.37 ms/op 1.05
Container {a,vec,b} - as struct x100000 93.005 us/op 94.094 us/op 0.99
Container {a,vec,b} - as tree x100000 103.32 ms/op 102.73 ms/op 1.01
Simple object binary -> struct 1.9850 us/op 2.1700 us/op 0.91
Simple object binary -> tree_backed 2.5220 us/op 2.4260 us/op 1.04
Simple object struct -> tree_backed 1.9140 us/op 1.8810 us/op 1.02
Simple object tree_backed -> struct 2.2310 us/op 2.3040 us/op 0.97
Simple object struct -> binary 2.1060 us/op 2.0280 us/op 1.04
Simple object tree_backed -> binary 3.9310 us/op 3.6710 us/op 1.07
aggregationBits binary -> struct 38.057 us/op 36.693 us/op 1.04
aggregationBits binary -> tree_backed 2.4400 us/op 2.2450 us/op 1.09
aggregationBits struct -> tree_backed 6.1160 us/op 5.2100 us/op 1.17
aggregationBits tree_backed -> struct 79.406 us/op 75.175 us/op 1.06
aggregationBits struct -> binary 4.3420 us/op 4.0150 us/op 1.08
aggregationBits tree_backed -> binary 5.5750 us/op 5.4260 us/op 1.03
List(BigInt) 1000 binary -> struct 399.53 us/op 394.70 us/op 1.01
List(BigInt) 1000 binary -> tree_backed 289.84 us/op 267.75 us/op 1.08
List(BigInt) 1000 struct -> tree_backed 324.25 us/op 284.83 us/op 1.14
List(BigInt) 1000 tree_backed -> struct 969.36 us/op 1.0120 ms/op 0.96
List(BigInt) 1000 struct -> binary 207.82 us/op 203.08 us/op 1.02
List(BigInt) 1000 tree_backed -> binary 51.657 us/op 47.752 us/op 1.08
List(BigInt) 10000 binary -> struct 3.9748 ms/op 3.9348 ms/op 1.01
List(BigInt) 10000 binary -> tree_backed 2.7012 ms/op 2.6522 ms/op 1.02
List(BigInt) 10000 struct -> tree_backed 2.8994 ms/op 2.7230 ms/op 1.06
List(BigInt) 10000 tree_backed -> struct 9.7723 ms/op 9.7461 ms/op 1.00
List(BigInt) 10000 struct -> binary 2.0094 ms/op 1.9471 ms/op 1.03
List(BigInt) 10000 tree_backed -> binary 377.69 us/op 381.39 us/op 0.99
List(BigInt) 100000 binary -> struct 47.173 ms/op 48.115 ms/op 0.98
List(BigInt) 100000 binary -> tree_backed 39.303 ms/op 40.694 ms/op 0.97
List(BigInt) 100000 struct -> tree_backed 38.638 ms/op 39.100 ms/op 0.99
List(BigInt) 100000 tree_backed -> struct 116.29 ms/op 120.95 ms/op 0.96
List(BigInt) 100000 struct -> binary 22.412 ms/op 20.790 ms/op 1.08
List(BigInt) 100000 tree_backed -> binary 4.6259 ms/op 4.8085 ms/op 0.96
struct - increase slot to 1000000 2.6528 ms/op 2.6810 ms/op 0.99
NumberUintType - increase slot to 1000000 1.3482 s/op 1.3898 s/op 0.97
Number64UintType - increase slot to 1000000 497.29 ms/op

by benchmarkbot/action

@twoeths twoeths requested a review from wemeetagain August 21, 2021 04:27
Copy link
Contributor

@dapplion dapplion left a comment

Choose a reason for hiding this comment

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

Looks really good! Just some minor comments

@twoeths twoeths requested a review from dapplion August 23, 2021 07:38
dapplion
dapplion previously approved these changes Aug 23, 2021
Copy link
Contributor

@dapplion dapplion left a comment

Choose a reason for hiding this comment

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

🔥

dapplion
dapplion previously approved these changes Aug 27, 2021
Add optional tree_* hash object methods to BasicType
Remove FieldInfo.isNumber64Type
Remove separate number64_* functions
Rename applyDeltas methods
Copy link
Member

@wemeetagain wemeetagain left a comment

Choose a reason for hiding this comment

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

I pushed a commit on top, let me know what you think. I'd like to add methods to serialize/deserialize from HashObject for all BasicType s eventually (Mainly will be applicable to BooleanType). Ideally, the HashObject path should be taken when eg: validators[i].slashed is checked or set.

Here's a list of things changed:

  • Add optional tree_* hash object methods to BasicType - this lets us check for the existence of the method instead of using FieldInfo.isNumber64Type, useful for the scenario above
  • Remove FieldInfo.isNumber64Type
  • Remove separate number64_* functions - we can always split these out if we use them in multiple places
  • Rename applyDeltas methods - remove the word Uint64 from these methods

@twoeths
Copy link
Contributor Author

twoeths commented Aug 28, 2021

I pushed a commit on top, let me know what you think. I'd like to add methods to serialize/deserialize from HashObject for all BasicType s eventually (Mainly will be applicable to BooleanType). Ideally, the HashObject path should be taken when eg: validators[i].slashed is checked or set.

Here's a list of things changed:

  • Add optional tree_* hash object methods to BasicType - this lets us check for the existence of the method instead of using FieldInfo.isNumber64Type, useful for the scenario above
  • Remove FieldInfo.isNumber64Type
  • Remove separate number64_* functions - we can always split these out if we use them in multiple places
  • Rename applyDeltas methods - remove the word Uint64 from these methods

Thanks @wemeetagain for all of this 👍

@twoeths twoeths merged commit 45620b7 into master Aug 28, 2021
@twoeths twoeths deleted the tuyen/Number64UintType branch August 28, 2021 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consume HashObject in ssz and new array method
3 participants