Validation benchmarks and performance improvements #1785

nc6 · 2020-08-14T10:01:32Z

This is a cleaned and rebased version of #1767.

It may be reviewed commit by commit. Particular attention should be paid in review to the TxOut pattern changes.

Much of the work here is due to @TimSheard, I've just tidied it up for review.

This is an additional set of constraints on the mock crypto for use in examples. Crucially, it is required whenever we rely on using the fake VRF algorithm.

shelley/chain-and-ledger/shelley-spec-ledger-test/shelley-spec-ledger-test.cabal

mrBliss · 2020-08-14T10:13:27Z

semantics/executable-spec/src/Control/Iterate/SetAlgebraInternal.hs

@@ -999,6 +999,7 @@ compute:: Exp t -> t
 compute (Base rep relation) = relation

 compute (Dom (Base SetR rel)) = rel
+compute (Dom (Base MapR x)) = Sett (Map.keysSet x)


This reminds me of #1775, but doesn't help (yet?) because STS.bbodySlots = eval (dom nesOsched) forces the evaluation when creating the BbodyEnv: https://github.com/input-output-hk/cardano-ledger-specs/blob/d0f7eded325c4e4b36e44b4b49fd75b0c83f00f3/shelley/chain-and-ledger/executable-spec/src/Shelley/Spec/Ledger/API/Validation.hs#L72

shelley/chain-and-ledger/executable-spec/src/Shelley/Spec/Ledger/DeserializeShort.hs

shelley/chain-and-ledger/executable-spec/shelley-spec-ledger.cabal

mrBliss · 2020-08-14T10:19:50Z

shelley/chain-and-ledger/executable-spec/src/Shelley/Spec/Ledger/TxData.hs

- addr = case deserialiseAddr (BSS.fromShort bs) of
- Nothing -> panic "viewCompactTxOut: impossible"
+ addr = case deserializeShortAddr bs of -- Try to deserialize a Shelley style Addr directly from ShortByteString
 Just (a :: Addr crypto) -> a
+ Nothing -> case deserialiseAddr (BSS.fromShort bs) of -- It is a Byron Address, try the more expensive route.
+ Nothing -> panic "viewCompactTxOut: impossible"
+ Just (a :: Addr crypto) -> a


Great! Is the goal to eventually let deserializeShortAddr support Byron addresses too?

These cover two areas: - Block generation (via 'genBlock') - Block and header application (in 'BenchValidation')

In particular the function 'viewCompactTxOut' which now tries to serialize the Address directly from a ShortByteString, and falls back to the expensive 'deserialiseAddr' only if that fails. Benchmarking shows that it almost always succeeds. This speeds up the 'applyBlockTransition' function about 10-12%. More speedup is possible because the new bottle neck is the function 'substring :: ShortByteString -> Int -> Int -> ShortByteString' which has an absolutely horrible implementation, it should use a single allocation and a memcopy operation, but instead it goes through 2 intermediate lists.

Work by Tim Sheard.

Highlights: * IntersectMBO/cardano-ledger#1784 * IntersectMBO/cardano-ledger#1785 * IntersectMBO/cardano-ledger#1779 * IntersectMBO/ouroboros-network#2512

mrBliss · 2020-08-15T17:00:23Z

I have had a go at optimising the remaining bottleneck described in 617d601: see #1786 (that PR has this PR as its target).

In my microbenchmark, this brings down the cost of deserialising a Shelley address in `viewCompactTxOut` from ~360ns to ~60ns. The `cloneByteArray` function was introduced in version 0.7.1.0 of the `primitive` package, so update the lower bound accordingly. I believe the old `primitive < 0.7` constraint in `cabal.project` was due to older versions of the `cborg` package having `>=0.5 && <0.7.1.0` as bounds for `primitive`, but `cborg-0.2.4.0` bumped the upper bound to 0.8. This will likely require similar changes to downstream repos.

The dependency on cborg was pinned to an unreleased revision to bring in <well-typed/cborg#223>. That PR has been included in the cborg 0.2.3.0, so we remove the dependency pin in favour of bumping the lower bound on the version. Instead of using 0.2.3 as the lower bound, bump it to 0.2.4, as this version is compatible with the last version of the `primitive` package, which will soon be needed by `cardano-ledger-specs`, see <IntersectMBO/cardano-ledger#1785>. Another advantage of using a released version is that the package will now be stored in the global cabal store and shared between projects (although that will also be the case for source dependencies in Cabal 3.4.0.0).

The dependency on cborg was pinned to an unreleased revision to bring in <well-typed/cborg#223>. That PR has been included in `cborg-0.2.3.0`, so we can remove the dependency pin in favour of bumping the lower version bound on the dependency. Instead of using 0.2.3 as the lower version bound, bump it to >= 0.2.4, as this version is compatible with the last version of the `primitive` package, which will soon be needed by `cardano-ledger-specs`, see <IntersectMBO/cardano-ledger#1785>. Another advantage of using a released version is that the package will now be stored in the global cabal store and shared between projects (although that will also be the case for source dependencies in Cabal 3.4.0.0).

JaredCorduan

LGTM!

Highlights: * IntersectMBO/cardano-ledger#1784 * IntersectMBO/cardano-ledger#1785 * IntersectMBO/cardano-ledger#1779 * IntersectMBO/ouroboros-network#2512

The dependency on cborg was pinned to an unreleased revision to bring in <well-typed/cborg#223>. That PR has been included in `cborg-0.2.3.0`, so we can remove the dependency pin in favour of bumping the lower version bound on the dependency. Instead of using 0.2.3 as the lower version bound, bump it to >= 0.2.4, as this version is compatible with the last version of the `primitive` package, which will soon be needed by `cardano-ledger-specs`, see <IntersectMBO/cardano-ledger#1785>. Another advantage of using a released version is that the package will now be stored in the global cabal store and shared between projects (although that will also be the case for source dependencies in Cabal 3.4.0.0).

mrBliss · 2020-08-17T18:18:27Z

FYI, using this branch + #1775, I get the following profiling hotspots when running consensus' db-validator (performs block reapplication and header validation) on all Shelley epochs upto 268:

COST CENTRE                      MODULE                                  SRC                                                             %time %alloc

verify                           Cardano.Crypto.VRF.Praos                src/Cardano/Crypto/VRF/Praos.hs:(388,1)-(397,33)                 46.9    0.1
verify                           Crypto.PubKey.Ed25519                   Crypto/PubKey/Ed25519.hs:(107,1)-(114,30)                         6.3    0.0
compute                          Control.Iterate.SetAlgebraInternal      src/Control/Iterate/SetAlgebraInternal.hs:(999,1)-(1106,33)       2.8    2.0
trans                            Control.State.Transition.Extended       src/Control/State/Transition/Extended.hs:253:1-36                 2.6    5.3
debugTracer                      Control.Tracer                          src/Control/Tracer.lhs:260:1-39                                   1.7    3.8
copyAndFreeze                    Data.ByteArray.Methods                  Data/ByteArray/Methods.hs:(237,1)-(240,21)                        1.4    2.3
liftSTS                          Control.State.Transition.Extended       src/Control/State/Transition/Extended.hs:259:1-30                 1.2    2.2
translateUTxOByronToShelley      Ouroboros.Consensus.Cardano.CanHardFork src/Ouroboros/Consensus/Cardano/CanHardFork.hs:(292,1)-(298,7)    1.2    1.4
compare                          Shelley.Spec.Ledger.TxData              src/Shelley/Spec/Ledger/TxData.hs:378:23-25                       1.1    0.0
deserialiseIncremental           Codec.CBOR.Read                         src/Codec/CBOR/Read.hs:(165,1)-(167,46)                           1.0    2.0
allocAndFreeze                   Data.ByteArray.Methods                  Data/ByteArray/Methods.hs:74:1-45                                 0.7    1.5
liftF                            Control.Monad.Free.Class                src/Control/Monad/Free/Class.hs:154:1-26                          0.7    1.5
cpure_NP.go                      Data.SOP.Strict                         src/Data/SOP/Strict.hs:(82,5)-(83,28)                             0.5    1.0
toBuilder                        Codec.CBOR.Write                        src/Codec/CBOR/Write.hs:(84,1)-(85,57)                            0.5    1.1
convert                          Data.ByteArray.Methods                  Data/ByteArray/Methods.hs:306:1-67                                0.5    1.3
getAddr                          Shelley.Spec.Ledger.Address             src/Shelley/Spec/Ledger/Address.hs:(295,1)-(307,73)               0.5    1.3
serializeEncoding                Cardano.Binary.Serialize                src/Cardano/Binary/Serialize.hs:(61,1)-(67,49)                    0.4    5.2
judgmentContext                  Control.State.Transition.Extended       src/Control/State/Transition/Extended.hs:263:1-36                 0.4    1.1
toBuilder                        Codec.CBOR.ByteArray.Sliced             src/Codec/CBOR/ByteArray/Sliced.hs:(87,1)-(100,31)                0.4    1.1
toCborError                      Cardano.Prelude.Error                   src/Cardano/Prelude/Error.hs:33:1-35                              0.3    1.9
toCBOR                           Shelley.Spec.Ledger.Address             src/Shelley/Spec/Ledger/Address.hs:462:3-38                       0.3    5.9
satisfy                          Text.Parsec.Char                        src/Text/Parsec/Char.hs:(147,1)-(149,71)                          0.2    1.9
compare                          Shelley.Spec.Ledger.TxData              src/Shelley/Spec/Ledger/TxData.hs:387:32-34                       0.2    1.2
serialiseAddr                    Shelley.Spec.Ledger.Address             src/Shelley/Spec/Ledger/Address.hs:152:1-49                       0.2    2.2
fromCompactTxId                  Cardano.Chain.UTxO.Compact              src/Cardano/Chain/UTxO/Compact.hs:(175,1)-(176,62)                0.1    5.8
fromCompactRedeemVerificationKey Cardano.Crypto.Signing.Redeem.Compact   src/Cardano/Crypto/Signing/Redeem/Compact.hs:(77,1)-(82,46)       0.1    3.9

My takeaway from this: there is no obvious nail we can hammer on. The two first entries are C functions. We can check whether compute is still doing something suboptimal, but it might as well be doing the best it can already.

I'm thinking about parallelising/pipelining header vs. block validation or batching it. Pipeling and batching would require a major redesign, as validation (header + block) is inherently sequential. Parallelising header and block validation is easier, but it would be limited to validating a single block's header in parallel to the validation of the block/body, so there's less to win. Note that that code is pure at the moment, we could keep it pure by using Control.Parallel.Strategies.

Highlights: * IntersectMBO/cardano-ledger#1784 * IntersectMBO/cardano-ledger#1785 * IntersectMBO/cardano-ledger#1779 * IntersectMBO/ouroboros-network#2512 * Remove unused source-repository-package cardano-sl-x509 (this package required the `ip < 1.5` constraint which conflicted with the `primitive >= 0.7.1.0` lower bound in `cardano-ledger-specs`). * Remove unused http-client dependency from stack.yaml * Remove duplicate from cabal.project

The dependency on cborg was pinned to an unreleased revision to bring in <well-typed/cborg#223>. That PR has been included in `cborg-0.2.3.0`, so we can remove the dependency pin in favour of bumping the lower version bound on the dependency. Instead of using 0.2.3 as the lower version bound, bump it to >= 0.2.4, as this version is compatible with the last version of the `primitive` package, which will soon be needed by `cardano-ledger-specs`, see <IntersectMBO/cardano-ledger#1785>. Another advantage of using a released version is that the package will now be stored in the global cabal store and shared between projects (although that will also be the case for source dependencies in Cabal 3.4.0.0).

Highlights: * IntersectMBO/cardano-ledger#1784 * IntersectMBO/cardano-ledger#1785 * IntersectMBO/cardano-ledger#1779 * IntersectMBO/cardano-ledger#1785 * IntersectMBO/cardano-ledger#1742 * IntersectMBO/ouroboros-network#2512 * Remove unused source-repository-package cardano-sl-x509 (this package required the `ip < 1.5` constraint which conflicted with the `primitive >= 0.7.1.0` lower bound in `cardano-ledger-specs`). * Remove unused http-client dependency from stack.yaml * Remove duplicate from cabal.project

1696: Update dependencies r=mrBliss a=mrBliss Highlights: * IntersectMBO/cardano-ledger#1784 * IntersectMBO/cardano-ledger#1785 * IntersectMBO/cardano-ledger#1779 * IntersectMBO/cardano-ledger#1785 * IntersectMBO/cardano-ledger#1742 * IntersectMBO/ouroboros-network#2512 Co-authored-by: Thomas Winant <thomas@well-typed.com> Co-authored-by: Samuel Leathers <samuel.leathers@iohk.io>

TimSheard · 2020-08-18T19:12:36Z

I aggree with your analysis. Everything that does a lookup in one of the ledger states Maps or Sets goes through 'compute' if that only uses 2% of the time, it is unlikely that could be improved enough to make a difference.

The dependency on cborg was pinned to an unreleased revision to bring in <well-typed/cborg#223>. That PR has been included in `cborg-0.2.3.0`, so we can remove the dependency pin in favour of bumping the lower version bound on the dependency. Instead of using 0.2.3 as the lower version bound, bump it to >= 0.2.4, as this version is compatible with the last version of the `primitive` package, which will soon be needed by `cardano-ledger-specs`, see <IntersectMBO/cardano-ledger#1785>. Another advantage of using a released version is that the package will now be stored in the global cabal store and shared between projects (although that will also be the case for source dependencies in Cabal 3.4.0.0).

1696: Update dependencies r=mrBliss a=mrBliss Highlights: * IntersectMBO/cardano-ledger#1784 * IntersectMBO/cardano-ledger#1785 * IntersectMBO/cardano-ledger#1779 * IntersectMBO/cardano-ledger#1785 * IntersectMBO/cardano-ledger#1742 * IntersectMBO/ouroboros-network#2512 Co-authored-by: Thomas Winant <thomas@well-typed.com> Co-authored-by: Samuel Leathers <samuel.leathers@iohk.io>

nc6 added 2 commits August 14, 2020 11:40

Bump cardano-base for cardano-crypto-praos

690cde5

Introduce 'ExMock'.

430a6f8

This is an additional set of constraints on the mock crypto for use in examples. Crucially, it is required whenever we rely on using the fake VRF algorithm.

nc6 requested review from JaredCorduan, TimSheard and redxaxder August 14, 2020 10:01

nc6 requested a review from uroboros as a code owner August 14, 2020 10:01

nc6 mentioned this pull request Aug 14, 2020

Bench validation #1767

Closed

mrBliss reviewed Aug 14, 2020

View reviewed changes

nc6 force-pushed the nc/benchValidation branch from 7fc147b to 646ff9d Compare August 14, 2020 10:51

nc6 added 3 commits August 14, 2020 13:57

Introduce new benchmarks.

a4408b6

These cover two areas: - Block generation (via 'genBlock') - Block and header application (in 'BenchValidation')

Add some additional type-specific optimisations.

bcadc46

Work by Tim Sheard.

nc6 force-pushed the nc/benchValidation branch from 646ff9d to bcadc46 Compare August 14, 2020 11:57

mrBliss added a commit to IntersectMBO/cardano-node that referenced this pull request Aug 14, 2020

Update dependencies

ec63644

Highlights: * IntersectMBO/cardano-ledger#1784 * IntersectMBO/cardano-ledger#1785 * IntersectMBO/cardano-ledger#1779 * IntersectMBO/ouroboros-network#2512

mrBliss mentioned this pull request Aug 14, 2020

Update dependencies IntersectMBO/cardano-node#1696

Merged

mrBliss mentioned this pull request Aug 17, 2020

Keep every snapshot in memory and remove the concept of snapEvery IntersectMBO/ouroboros-network#2459

Merged

mrBliss mentioned this pull request Aug 17, 2020

Use a released version of cborg IntersectMBO/cardano-node#1711

Closed

JaredCorduan approved these changes Aug 17, 2020

View reviewed changes

nc6 merged commit 7e2e18f into master Aug 17, 2020

nc6 deleted the nc/benchValidation branch August 17, 2020 14:07

mrBliss added a commit to IntersectMBO/cardano-node that referenced this pull request Aug 17, 2020

Update dependencies

905226f

Highlights: * IntersectMBO/cardano-ledger#1784 * IntersectMBO/cardano-ledger#1785 * IntersectMBO/cardano-ledger#1779 * IntersectMBO/ouroboros-network#2512

mrBliss mentioned this pull request Aug 18, 2020

Introduce header revalidation IntersectMBO/ouroboros-network#2520

Closed

nc6 mentioned this pull request Aug 25, 2020

Block Validation Performance #1750

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validation benchmarks and performance improvements #1785

Validation benchmarks and performance improvements #1785

nc6 commented Aug 14, 2020

mrBliss Aug 14, 2020

mrBliss Aug 14, 2020

mrBliss commented Aug 15, 2020 •

edited

Loading

JaredCorduan left a comment

mrBliss commented Aug 17, 2020

TimSheard commented Aug 18, 2020

Validation benchmarks and performance improvements #1785

Validation benchmarks and performance improvements #1785

Conversation

nc6 commented Aug 14, 2020

mrBliss Aug 14, 2020

Choose a reason for hiding this comment

mrBliss Aug 14, 2020

Choose a reason for hiding this comment

mrBliss commented Aug 15, 2020 • edited Loading

JaredCorduan left a comment

Choose a reason for hiding this comment

mrBliss commented Aug 17, 2020

TimSheard commented Aug 18, 2020

mrBliss commented Aug 15, 2020 •

edited

Loading