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
Benchmark the production code fully rather than some arbitrary thing #5409
Benchmark the production code fully rather than some arbitrary thing #5409
Conversation
/benchmark validation-full |
Click here to check the status of your benchmark. |
Comparing benchmark results of ' validation-full' on '7f3fabd2d1' (base) and '3430c60cf4' (PR) Results table
|
3430c60
to
dfdc6b5
Compare
^ yay, noise. |
-- strictify and "short" the result cbor to create a real `SerialisedScript` | ||
!benchScript = force . serialiseUPLC $ UPLC.Program () ver term | ||
eval script = do | ||
either (error . show) (\_ -> ()) . snd $ evaluateScriptRestricting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use i think const () . PlutusPrelude.unsafeFromRight
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that'd return ()
regardless of whether we get Left
or Right
and we do want to throw on Left
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oups yeah
(term, args) = peelDataArguments body | ||
-- strictify and "short" the result cbor to create a real `SerialisedScript` | ||
!benchScript = force . serialiseUPLC $ UPLC.Program () ver term | ||
eval script = do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do
looks redundant here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, thanks, fixed.
dfdc6b5
to
e84de84
Compare
Should we worry about this for the nofib benchmarks too? I'm getting confused because we have lots of different benchmarks that all seem to have their own ways of evaluating terms. |
This is a continuation of #5200, which updated
BenchCek.hs
to use the correct evaluation context. This one updatesBenchFull.hs
.