{"action":"created","issue":{"url":"https://api.github.com/repos/input-output-hk/cardano-sl/issues/2324","repository_url":"https://api.github.com/repos/input-output-hk/cardano-sl","labels_url":"https://api.github.com/repos/input-output-hk/cardano-sl/issues/2324/labels{/name}","comments_url":"https://api.github.com/repos/input-output-hk/cardano-sl/issues/2324/comments","events_url":"https://api.github.com/repos/input-output-hk/cardano-sl/issues/2324/events","html_url":"https://github.com/input-output-hk/cardano-sl/pull/2324","id":288219775,"number":2324,"title":"[CSL-2043] [CSL-2149] Block serialization/deserialization improvements (with benchmarks)","user":{"login":"avieth","id":1420272,"avatar_url":"https://avatars2.githubusercontent.com/u/1420272?v=4","gravatar_id":"","url":"https://api.github.com/users/avieth","html_url":"https://github.com/avieth","followers_url":"https://api.github.com/users/avieth/followers","following_url":"https://api.github.com/users/avieth/following{/other_user}","gists_url":"https://api.github.com/users/avieth/gists{/gist_id}","starred_url":"https://api.github.com/users/avieth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/avieth/subscriptions","organizations_url":"https://api.github.com/users/avieth/orgs","repos_url":"https://api.github.com/users/avieth/repos","events_url":"https://api.github.com/users/avieth/events{/privacy}","received_events_url":"https://api.github.com/users/avieth/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":70,"created_at":"2018-01-12T19:18:53Z","updated_at":"2018-02-20T15:41:00Z","closed_at":null,"author_association":"COLLABORATOR","pull_request":{"url":"https://api.github.com/repos/input-output-hk/cardano-sl/pulls/2324","html_url":"https://github.com/input-output-hk/cardano-sl/pull/2324","diff_url":"https://github.com/input-output-hk/cardano-sl/pull/2324.diff","patch_url":"https://github.com/input-output-hk/cardano-sl/pull/2324.patch"},"body":"Title recently changes to reflect the fact that this should eventually be merged.\r\nAll that remains to do is replace the various verifications which were formerly done in the deserialization instance.\r\n\r\n# Block serialization benchmarks\r\n\r\n## Purpose\r\n\r\nTo prove or disprove, with evidence, my claim that verification in\r\ndeserialization is a significant performance problem, and to discover the\r\nextent to which recent optimizations to `Digest` marshalling will offer any\r\nimprovements in deserialization and serialization alike.\r\n\r\nAll this in the context of increased pressure on our relays due to ADA hype,\r\nand the longstanding issue of a painfully slow initial wallet sync.\r\nImprovements in serialization and deserialization performance could reduce\r\nrelay load as well as block sync latency.\r\n\r\n## Test setup\r\n\r\nSee `block/bench/Block.hs`.\r\n\r\nThe `Arbitrary` instances for `MainBlock` is used to come up with test subjects.\r\nThe deterministic `QCGen` from `QuickCheck` is seeded according to the\r\nenvironment variable `SEED` and the size of the subjects is determined by the\r\nenvironment variable `SIZE`.\r\n\r\nThe uses of `makeSmall` from `cardano-sl-util` were banished. This term\r\ntakes the square root of a third of the size, and was applied to, among other\r\nthings, block bodies. It in essence corrupted the data.\r\n\r\nThe benchmark suite includes deserialization and serialization of the block,\r\nblock header, and each of their components, including all components of the\r\nblock body (payloads).\r\n\r\nThe suite must be compiled against different revisions of the rest of\r\ncardano-sl, corresponding to the three cases:\r\n\r\n  - Verification on deserialization, no further optimizations (3451d1b32d6f86a98ca7f1cc9c1de34ac4a32114).\r\n  - No verification on deserialization, no further optimizations (3451d1b32d6f86a98ca7f1cc9c1de34ac4a32114).\r\n  - No verification on deserialization, various optimizations (47652100600aff15da3569b67d520a697315e911).\r\n\r\nThe \"various optimizations\" include:\r\n\r\n  - More efficient `Digest`/`ByteString` conversion (requires new basement,\r\n    memory, cryptonite (unreleased) packages).\r\n  - Fewer uses of `LBS.toStrict`.\r\n  - Custom `Builder` strategies for serialization.\r\n\r\n`criterion` is used to gather data. With my recent patches (tip of mainline\r\nat the moment, but unreleased) it now properly collects RTS numbers and so we\r\ncan regress bytes allocated over iterations:\r\n\r\n```\r\nSEED=0 SIZE=32 ./block/bench/Block --regress allocated:iters +RTS -T -RTS\r\n```\r\n\r\nAll exectuables compiled with `-O2`.\r\nAll tests were run with SEED=4, SIZE=32, giving a block of size 77,496 bytes.\r\n\r\n## Results\r\n\r\n- Significant improvements in deserialization time when dropping validation:\r\n  - Overall time cut by one third. No significant change in allocations overall.\r\n  - SSC payload deserializes in one tenth of the time, with significantly fewer\r\n    allocations.\r\n  - Update payload deserializes in one tenth of the time, also with significantly\r\n    fewer allocations (less than half).\r\n  - Delegation payload deserializes in one one hundreth of the time, with less\r\n    than 20% of allocations.\r\n  - No significant change in `TxPayload` deserialization time (that's because\r\n    it still has to create the merkle tree).\r\n  - Header deserializes in less than 20% of the previous time, roughly half of\r\n    the allocations.\r\n  - No significant change in body proof, consensus, or extra data.\r\n\r\n- Significant improvements in serialization and deserialization with extra\r\n  optimizations:\r\n  - Serialization of the whole block down from 2.067ms to 1.297ms, allocations\r\n    down from 6.968m to 3.666m.\r\n  - Deserialization of the whole block down form 8.580ms to 4.920ms, allocations\r\n    down from 11.2m to 6.21m.\r\n  - `TxPayload` serialize greatly improved but remains by far the most\r\n    expensive component of the body. Hypothesis: `Address` decoding is still\r\n    very expensive, as is construction of the merkle tree.\r\n\r\nTODO more detailed analysis.\r\n\r\nIf you want to run these yourself, you'll need, in addition to unreleased `criterion`, an unreleased `cryptonite` [revision](https://github.com/haskell-crypto/cryptonite/pull/211) and local changes to `cardano-crypto` which I can provide if you are interested. Alternatively, use bcb1a1ac2e406ae425e27fcc080ec2ae968beda6 which does not need the `cardano-crypto` changes."},"comment":{"url":"https://api.github.com/repos/input-output-hk/cardano-sl/issues/comments/367018106","html_url":"https://github.com/input-output-hk/cardano-sl/pull/2324#issuecomment-367018106","issue_url":"https://api.github.com/repos/input-output-hk/cardano-sl/issues/2324","id":367018106,"user":{"login":"avieth","id":1420272,"avatar_url":"https://avatars2.githubusercontent.com/u/1420272?v=4","gravatar_id":"","url":"https://api.github.com/users/avieth","html_url":"https://github.com/avieth","followers_url":"https://api.github.com/users/avieth/followers","following_url":"https://api.github.com/users/avieth/following{/other_user}","gists_url":"https://api.github.com/users/avieth/gists{/gist_id}","starred_url":"https://api.github.com/users/avieth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/avieth/subscriptions","organizations_url":"https://api.github.com/users/avieth/orgs","repos_url":"https://api.github.com/users/avieth/repos","events_url":"https://api.github.com/users/avieth/events{/privacy}","received_events_url":"https://api.github.com/users/avieth/received_events","type":"User","site_admin":false},"created_at":"2018-02-20T15:41:00Z","updated_at":"2018-02-20T15:41:00Z","author_association":"COLLABORATOR","body":"> If polluted means a lot of logging, then no, stdout won't be polluted. If 2 lines of logs are critical to you, then yes, stdout will be polluted.\r\n\r\nWell, I mean, I want criterion to have full reign over stdout, as that's where it puts its results. So yes, 2 lines is pollution."}}
{"actor":{"display_login":"avieth"}}