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

More measures post restoration benchmarks + SeqAnyState #2084

Merged
merged 7 commits into from
Sep 2, 2020

Conversation

KtorZ
Copy link
Member

@KtorZ KtorZ commented Aug 27, 2020

Issue Number

#2032

Overview

  • d820f36 make proportion more fine-grained to allow fractions of percent
  • ce3f2a4 define a 'SeqAnyState' analogous to the 'RndAnyState' for benchmarks
  • 02aba16 perform additional checks after restoration benchmarks
  • 793de1c extend restoration benchmarks with a special 1% random wallet

Comments

On the testnet (although I messed up and the rnd % benchmark are actually doing 1%, 2% and 4% restorations!)

All results:
BenchResults:
  qualifier: Seq Empty Wallet
  restorationTime: 521.8 s
  listingAddressesTime: 15.90 ms
  estimatingFeesTime: 2.015 s
  utxoStatistics:
    = Total value of -0 lovelace across 0 UTxOs
     ... 10                0
     ... 100               0
     ... 1000              0
     ... 10000             0
     ... 100000            0
     ... 1000000           0
     ... 10000000          0
     ... 100000000         0
     ... 1000000000        0
     ... 10000000000       0
     ... 100000000000      0
     ... 1000000000000     0
     ... 10000000000000    0
     ... 100000000000000   0
     ... 1000000000000000  0
     ... 10000000000000000 0
     ... 45000000000000000 0

BenchResults:
  qualifier: Rnd Empty Wallet
  restorationTime: 377.9 s
  listingAddressesTime: 989.2 μs
  estimatingFeesTime: 32.40 s
  utxoStatistics:
    = Total value of -0 lovelace across 0 UTxOs
     ... 10                0
     ... 100               0
     ... 1000              0
     ... 10000             0
     ... 100000            0
     ... 1000000           0
     ... 10000000          0
     ... 100000000         0
     ... 1000000000        0
     ... 10000000000       0
     ... 100000000000      0
     ... 1000000000000     0
     ... 10000000000000    0
     ... 100000000000000   0
     ... 1000000000000000  0
     ... 10000000000000000 0
     ... 45000000000000000 0

BenchResults:
  qualifier: Rnd 0.1% Wallet
  restorationTime: 496.8 s
  listingAddressesTime: 72.31 ms
  estimatingFeesTime: 218.5 ms
  utxoStatistics:
    = Total value of 34915991397627 lovelace across 500 UTxOs
     ... 10                1
     ... 100               14
     ... 1000              1
     ... 10000             5
     ... 100000            36
     ... 1000000           16
     ... 10000000          12
     ... 100000000         28
     ... 1000000000        21
     ... 10000000000       138
     ... 100000000000      216
     ... 1000000000000     12
     ... 10000000000000    0
     ... 100000000000000   0
     ... 1000000000000000  0
     ... 10000000000000000 0
     ... 45000000000000000 0

BenchResults:
  qualifier: Rnd 0.2% Wallet
  restorationTime: 698.7 s
  listingAddressesTime: 643.8 ms
  estimatingFeesTime: 317.9 ms
  utxoStatistics:
    = Total value of 80967721697473 lovelace across 921 UTxOs
     ... 10                1
     ... 100               20
     ... 1000              1
     ... 10000             9
     ... 100000            64
     ... 1000000           28
     ... 10000000          19
     ... 100000000         42
     ... 1000000000        38
     ... 10000000000       246
     ... 100000000000      426
     ... 1000000000000     26
     ... 10000000000000    1
     ... 100000000000000   0
     ... 1000000000000000  0
     ... 10000000000000000 0
     ... 45000000000000000 0

BenchResults:
  qualifier: Rnd 0.4% Wallet
  restorationTime: 1016 s
  listingAddressesTime: 1.500 s
  estimatingFeesTime: 525.2 ms
  utxoStatistics:
    = Total value of 190005683160290 lovelace across 1894 UTxOs
     ... 10                2
     ... 100               45
     ... 1000              2
     ... 10000             12
     ... 100000            147
     ... 1000000           65
     ... 10000000          87
     ... 100000000         106
     ... 1000000000        93
     ... 10000000000       500
     ... 100000000000      772
     ... 1000000000000     59
     ... 10000000000000    3
     ... 100000000000000   1
     ... 1000000000000000  0
     ... 10000000000000000 0
     ... 45000000000000000 0

BenchResults:
  qualifier: Seq 0.1% Wallet
  restorationTime: 458.2 s
  listingAddressesTime: 25.73 ms
  estimatingFeesTime: 27.27 ms
  utxoStatistics:
    = Total value of 2912204907552 lovelace across 38 UTxOs
     ... 10                0
     ... 100               0
     ... 1000              0
     ... 10000             1
     ... 100000            0
     ... 1000000           2
     ... 10000000          1
     ... 100000000         2
     ... 1000000000        2
     ... 10000000000       11
     ... 100000000000      18
     ... 1000000000000     1
     ... 10000000000000    0
     ... 100000000000000   0
     ... 1000000000000000  0
     ... 10000000000000000 0
     ... 45000000000000000 0

BenchResults:
  qualifier: Seq 0.2% Wallet
  restorationTime: 510.7 s
  listingAddressesTime: 35.70 ms
  estimatingFeesTime: 39.94 ms
  utxoStatistics:
    = Total value of 6462355327839 lovelace across 86 UTxOs
     ... 10                1
     ... 100               1
     ... 1000              1
     ... 10000             1
     ... 100000            2
     ... 1000000           3
     ... 10000000          1
     ... 100000000         4
     ... 1000000000        2
     ... 10000000000       26
     ... 100000000000      42
     ... 1000000000000     2
     ... 10000000000000    0
     ... 100000000000000   0
     ... 1000000000000000  0
     ... 10000000000000000 0
     ... 45000000000000000 0

BenchResults:
  qualifier: Seq 0.4% Wallet
  restorationTime: 551.5 s
  listingAddressesTime: 61.45 ms
  estimatingFeesTime: 66.49 ms
  utxoStatistics:
    = Total value of 15179718492947 lovelace across 219 UTxOs
     ... 10                1
     ... 100               3
     ... 1000              1
     ... 10000             3
     ... 100000            8
     ... 1000000           4
     ... 10000000          6
     ... 100000000         19
     ... 1000000000        14
     ... 10000000000       59
     ... 100000000000      96
     ... 1000000000000     5
     ... 10000000000000    0
     ... 100000000000000   0
     ... 1000000000000000  0
     ... 10000000000000000 0
     ... 45000000000000000 0

@KtorZ KtorZ added the RESOLVING ISSUE Mark a PR as resolving issues, for auto-generated CHANGELOG label Aug 27, 2020
@KtorZ KtorZ requested a review from Anviking August 27, 2020 14:42
@KtorZ KtorZ self-assigned this Aug 27, 2020
Comment on lines 308 to 309
-- recognize as ours. It is expressed in tenth of percents, so "1" means 0.1%,
-- "10" means 1% and 1000 means 100%.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
-- recognize as ours. It is expressed in tenth of percents, so "1" means 0.1%,
-- "10" means 1% and 1000 means 100%.
-- recognize as ours. It is expressed in tenths of a percent, so "1" means 0.1%,
-- "10" means 1%, and 1000 means 100%.

There is also per mille, but it is rarely used in English, so I wouldn't recommend it. 😸

Comment on lines 765 to 766
-- recognize as ours. It is expressed in tenth of percents, so "1" means 0.1%,
-- "10" means 1% and 1000 means 100%.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
-- recognize as ours. It is expressed in tenth of percents, so "1" means 0.1%,
-- "10" means 1% and 1000 means 100%.
-- recognize as ours. It is expressed in tenths of a percent, so "1" means 0.1%,
-- "10" means 1%, and 1000 means 100%.

| otherwise =
(False, st)
where
p = floor (double (maxBound :: Word32) * double (natVal (Proxy @p)) / 1000)
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this is a bit easier to read:

Suggested change
p = floor (double (maxBound :: Word32) * double (natVal (Proxy @p)) / 1000)
p = floor
$ double (maxBound :: Word32)
* double (natVal (Proxy @p))
/ 1000

Copy link
Member Author

Choose a reason for hiding this comment

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

I disagree :), although I admit this could be more readable. What about:

        p = floor (double sup * double (natVal (Proxy @p)) / 1000)
          where
            sup = maxBound :: Word32 

Copy link
Member

@jonathanknowles jonathanknowles Sep 1, 2020

Choose a reason for hiding this comment

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

What about:

        p = floor (double sup * double (natVal (Proxy @p)) / 1000)
          where
            sup = maxBound :: Word32 

I think this is an improvement. I'm guessing sup is short for "superior" ("borne supérieure")?

Perhaps we could also give natVal (Proxy @p) a name, and then we'd have:

        p = floor ((double sup * double den) / 1000)
          where
            sup = maxBound :: Word32
            den = natVal (Proxy @p) 

Or:

        p = floor ((double n * double d) / 1000)
          where
            n = maxBound :: Word32
            d = natVal (Proxy @p) 

(where n and d are numerator and denominator, respectively).

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, although I think the line is already very readable :)

Comment on lines 738 to 739
-- it discover addresses based on an arbitrary ratio instead of respecting the
-- BIP-44 discovery.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
-- it discover addresses based on an arbitrary ratio instead of respecting the
-- BIP-44 discovery.
-- it discovers addresses based on an arbitrary ratio instead of respecting
-- BIP-44 discovery.

@KtorZ KtorZ force-pushed the KtorZ/2032/more-measures-post-restore-benchmark branch from 7306a33 to 8622066 Compare August 31, 2020 08:57
@KtorZ
Copy link
Member Author

KtorZ commented Aug 31, 2020

@jonathanknowles comments' typos fixed and made the definition of p more readable!

-- we ought to simply recognize as ours. So, giving @5 means that 5% of the
-- entire address space of the network will be considered ours, picked randomly.
-- The type parameter is expected to be a ratio of addresses we ought to simply
-- recognize as ours. It is expressed in tenths of percent, so "1" means 0.1%,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
-- recognize as ours. It is expressed in tenths of percent, so "1" means 0.1%,
-- recognize as ours. It is expressed in tenths of a percent, so "1" means 0.1%,

-- seed.
--
-- The type parameter is expected to be a ratio of addresses we ought to simply
-- recognize as ours. It is expressed in tenths of percent, so "1" means 0.1%,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
-- recognize as ours. It is expressed in tenths of percent, so "1" means 0.1%,
-- recognize as ours. It is expressed in tenths of a percent, so "1" means 0.1%,

| otherwise =
(False, st)
where
p = floor (double (maxBound :: Word32) * double (natVal (Proxy @p)) / 1000)
Copy link
Member

@jonathanknowles jonathanknowles Sep 1, 2020

Choose a reason for hiding this comment

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

What about:

        p = floor (double sup * double (natVal (Proxy @p)) / 1000)
          where
            sup = maxBound :: Word32 

I think this is an improvement. I'm guessing sup is short for "superior" ("borne supérieure")?

Perhaps we could also give natVal (Proxy @p) a name, and then we'd have:

        p = floor ((double sup * double den) / 1000)
          where
            sup = maxBound :: Word32
            den = natVal (Proxy @p) 

Or:

        p = floor ((double n * double d) / 1000)
          where
            n = maxBound :: Word32
            d = natVal (Proxy @p) 

(where n and d are numerator and denominator, respectively).

Copy link
Member

@jonathanknowles jonathanknowles left a comment

Choose a reason for hiding this comment

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

@jonathanknowles comments' typos fixed and made the definition of p more readable!

Thanks for fixing these! I added a couple more suggestions (as I think there is still an "a" missing from a couple of these comments).

Copy link
Collaborator

@Anviking Anviking left a comment

Choose a reason for hiding this comment

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

lgtm

lib/shelley/bench/Restore.hs Outdated Show resolved Hide resolved
lib/shelley/bench/Restore.hs Outdated Show resolved Hide resolved
  This wallet is very analogous to the existing any% wallet scheme we designed a while ago, with a subtle difference: it is built __on top of__ the 'RndState' and, as a result, does perform the same database operation and addresses management as the standard random wallets. So the benchmark results obtained from this are much closer to what an actual random wallet of the same size would look like.
  In particular, we check for the time needed to list addresses on a wallet, as well as the time needed to perform a fee estimation. This gives an order of magnitude of how long these functions take on large wallets.
  This make sure that benchmarks reflects a real-setup with a bit more fidelity by also storing and retrieving the address space.
@KtorZ KtorZ force-pushed the KtorZ/2032/more-measures-post-restore-benchmark branch from be93880 to 309874f Compare September 2, 2020 13:56
@KtorZ
Copy link
Member Author

KtorZ commented Sep 2, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 2, 2020

Canceled

@KtorZ
Copy link
Member Author

KtorZ commented Sep 2, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 2, 2020

Build succeeded

@iohk-bors iohk-bors bot merged commit db981dd into master Sep 2, 2020
@iohk-bors iohk-bors bot deleted the KtorZ/2032/more-measures-post-restore-benchmark branch September 2, 2020 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RESOLVING ISSUE Mark a PR as resolving issues, for auto-generated CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants