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

Add an MVP of a benchmarks comparison tool #161

Merged
merged 3 commits into from
Jun 28, 2023

Conversation

dnadales
Copy link
Member

@dnadales dnadales commented Jun 16, 2023

Based on the requirements gathered in #142, Closes #153

See the documentation in the Main module for more details.

@dnadales
Copy link
Member Author

dnadales commented Jun 16, 2023

For the time being this tool simply outputs the top 10 improvements and degradation wrt the baseline version. Sample output comparing main (50b7fbf7721659092b2e8cba6c5503daf3392c5e) vs cardano-0.5.0.0:

Comparison for mut_forecast
"Distance treshold exceeded!"
Top 10 measurements smaller than baseline (main)
"slot", "mut_forecast"
474934.0, -0.9454545454545454
52746.0, -0.9411764705882353
728477.0, -0.9384615384615385
240373.0, -0.937984496124031
832733.0, -0.9327731092436975
353359.0, -0.9322033898305084
525315.0, -0.9322033898305084
69640.0, -0.9310344827586207
851751.0, -0.9310344827586207
859652.0, -0.9302325581395349
Top 10 measurements larger than baseline (main)
"slot", "mut_forecast"
109289.0, 51.5
710153.0, 51.333333333333336
690864.0, 51.25
574362.0, 48.0
160136.0, 34.5
643656.0, 19.625
575318.0, 19.125
14694.0, 18.125
570092.0, 15.125
437150.0, 14.666666666666666
Comparison for mut_blockApply
"Distance treshold exceeded!"
Top 10 measurements smaller than baseline (main)
"slot", "mut_blockApply"
1248.0, -0.87248322147651
259086.0, -0.8703703703703703
279584.0, -0.8670886075949367
600631.0, -0.8666666666666667
321622.0, -0.863013698630137
161736.0, -0.8613138686131386
398185.0, -0.8601398601398601
22692.0, -0.859375
641784.0, -0.8592592592592593
235546.0, -0.8562091503267973
Top 10 measurements larger than baseline (main)
"slot", "mut_blockApply"
734380.0, 20.761904761904763
558421.0, 18.2
334686.0, 16.714285714285715
671255.0, 15.857142857142858
168744.0, 15.45
344760.0, 15.333333333333334
183762.0, 14.761904761904763
754399.0, 12.428571428571429
999937.0, 8.65
9263.0, 8.578947368421053

This tool also plots the outliers, eg:

image

@dnadales dnadales marked this pull request as ready for review June 20, 2023 10:01
@dnadales dnadales requested a review from a team as a code owner June 20, 2023 10:01
Comment on lines 590 to 612
hs-source-dirs: app
main-is: beacon.hs
build-depends:
, ansi-terminal
, base
, bytestring
, cassava
, Chart
, Chart-cairo
, containers
, directory
, extra
, process
, statistics
, text
, vector
, vector-algorithms

ghc-options:
-Wall -Wcompat -Wincomplete-uni-patterns
-Wincomplete-record-updates -Wpartial-fields -Widentities
-Wredundant-constraints -Wmissing-export-lists -Wunused-packages
-Wno-unticked-promoted-constructors
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
hs-source-dirs: app
main-is: beacon.hs
build-depends:
, ansi-terminal
, base
, bytestring
, cassava
, Chart
, Chart-cairo
, containers
, directory
, extra
, process
, statistics
, text
, vector
, vector-algorithms
ghc-options:
-Wall -Wcompat -Wincomplete-uni-patterns
-Wincomplete-record-updates -Wpartial-fields -Widentities
-Wredundant-constraints -Wmissing-export-lists -Wunused-packages
-Wno-unticked-promoted-constructors
import: common-exe
hs-source-dirs: app
main-is: beacon.hs
build-depends:
, ansi-terminal
, base
, bytestring
, cassava
, Chart
, Chart-cairo
, containers
, directory
, extra
, process
, statistics
, text
, vector
, vector-algorithms

@dnadales dnadales force-pushed the dnadales/153-mvp-benchmarks-comparison-tool branch from ee01711 to 68fb65a Compare June 26, 2023 10:04
@dnadales dnadales enabled auto-merge June 26, 2023 10:04
@dnadales
Copy link
Member Author

I ran the bench comparison tool through the chain I have locally and got some interesting results. Even though I did not perform statistical analysis, we can already see that the latest version of Consensus + Ledger got considerably faster (which I think we expected given the work Ledger did in the incremental rewards calculations ). I would guess that we can also detect serious regressions in this way. Version A is 0.6 and version B is 0.5.

image

@dnadales dnadales force-pushed the dnadales/153-mvp-benchmarks-comparison-tool branch 2 times, most recently from 4dd8692 to 3e132e7 Compare June 26, 2023 10:51
@dnadales dnadales force-pushed the dnadales/153-mvp-benchmarks-comparison-tool branch from 3e132e7 to 4c51b9e Compare June 27, 2023 12:27
@dnadales dnadales force-pushed the dnadales/153-mvp-benchmarks-comparison-tool branch from 556a93d to 4c2e65c Compare June 28, 2023 08:45
@dnadales
Copy link
Member Author

The tool does not build with ghc 9.6.1. This is the error I get after allowing a newer mtl version:

Failed to build Chart-1.9.4.
Build log (
/home/damian/.cabal/logs/ghc-9.6.1/Chart-1.9.4-f9a041ebf4cf01594c3d3f0db4634a9ceb2be3ea3541f84fe4dbcd42c6582b0e.log
):
Configuring library for Chart-1.9.4..
Preprocessing library for Chart-1.9.4..
Building library for Chart-1.9.4..
[ 1 of 37] Compiling Graphics.Rendering.Chart.Geometry ( Graphics/Rendering/Chart/Geometry.hs, dist/build/Graphics/Rendering/Chart/Geometry.o, dist/build/Graphics/Rendering/Chart/Geometry.dyn_o )

Graphics/Rendering/Chart/Geometry.hs:56:1: warning: [-Wunused-imports]
    The import of ‘Data.Monoid’ is redundant
      except perhaps to import instances from ‘Data.Monoid’
    To import instances alone, use: import Data.Monoid()
   |
56 | import Data.Monoid (Monoid (..))
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Graphics/Rendering/Chart/Geometry.hs:57:1: warning: [-Wunused-imports]
    The import of ‘Data.Semigroup’ is redundant
      except perhaps to import instances from ‘Data.Semigroup’
    To import instances alone, use: import Data.Semigroup()
   |
57 | import Data.Semigroup (Semigroup(..))
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[ 2 of 37] Compiling Graphics.Rendering.Chart.Backend.Types ( Graphics/Rendering/Chart/Backend/Types.hs, dist/build/Graphics/Rendering/Chart/Backend/Types.o, dist/build/Graphics/Rendering/Chart/Backend/Types.dyn_o )
[ 3 of 37] Compiling Graphics.Rendering.Chart.Backend.Impl ( Graphics/Rendering/Chart/Backend/Impl.hs, dist/build/Graphics/Rendering/Chart/Backend/Impl.o, dist/build/Graphics/Rendering/Chart/Backend/Impl.dyn_o )

Graphics/Rendering/Chart/Backend/Impl.hs:134:19: error: [GHC-88464]
    Variable not in scope:
      liftM
        :: (AlignmentFns -> AlignmentFn)
           -> ProgramT ChartBackendInstr m1 AlignmentFns
           -> BackendProgram (Point -> Point)
    Suggested fix:
      Perhaps use ‘lift’ (imported from Control.Monad.Reader)
    |
134 | getPointAlignFn = liftM afPointAlignFn (singleton GetAlignments)
    |                   ^^^^^

Graphics/Rendering/Chart/Backend/Impl.hs:138:19: error: [GHC-88464]
    Variable not in scope:
      liftM
        :: (AlignmentFns -> AlignmentFn)
           -> ProgramT ChartBackendInstr m0 AlignmentFns
           -> BackendProgram (Point -> Point)
    Suggested fix:
      Perhaps use ‘lift’ (imported from Control.Monad.Reader)
    |
138 | getCoordAlignFn = liftM afCoordAlignFn (singleton GetAlignments)
    |                   ^^^^^
[13 of 37] Compiling Graphics.Rendering.Chart.Utils ( Graphics/Rendering/Chart/Utils.hs, dist/build/Graphics/Rendering/Chart/Utils.o, dist/build/Graphics/Rendering/Chart/Utils.dyn_o )
[31 of 37] Compiling Numeric.Histogram ( Numeric/Histogram.hs, dist/build/Numeric/Histogram.o, dist/build/Numeric/Histogram.dyn_o )
Error: cabal: Failed to build Chart-1.9.4 (which is required by exe:beacon
from ouroboros-consensus-cardano-0.6.1.0). See the build log above for
details.

So the only option I see is to upstream a PR to Chart so that it builds with 9.6.1.

@dnadales dnadales added this pull request to the merge queue Jun 28, 2023
Merged via the queue into main with commit 601d9c2 Jun 28, 2023
11 of 12 checks passed
@dnadales dnadales deleted the dnadales/153-mvp-benchmarks-comparison-tool branch June 28, 2023 14:41
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.

Create a first prototype based on the requirements stated in #142
2 participants