[Benchmark] Pull 'force' out of 'bench' #7177
Open
+109
−101
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
effectfully
added
bug
Benchmarks
No Changelog Required
|
|
/benchmark nofib |
1 similar comment
|
/benchmark nofib |
|
/benchmark validation |
1 similar comment
|
/benchmark validation |
|
Click here to check the status of your benchmark. |
effectfully
force-pushed
the
effectfully/benchmark/force-outside-of-bench
branch
from
28fe3e9 to
3ce4366
Compare
|
Click here to check the status of your benchmark. |
effectfully
commented
Jul 2, 2025
| benchTermAgdaCek :: String -> Term -> Benchmark | ||
| benchTermAgdaCek name term = | ||
| let !term' = force term | ||
| in bench name $ whnf unsafeRunAgdaCek term' |
There was a problem hiding this comment.
Added force and changed nf to whnf for consistency.
Comment on lines
-137
to
+140
| benchTermCek :: LedgerApi.EvaluationContext -> Term -> Benchmarkable | ||
| benchTermCek evalCtx term = | ||
| benchTermCek :: String -> LedgerApi.EvaluationContext -> Term -> Benchmark | ||
| benchTermCek name evalCtx term = | ||
| let !term' = force term | ||
| in whnf (evaluateCekForBench evalCtx) term' | ||
| in bench name $ whnf (evaluateCekForBench evalCtx) term' |
There was a problem hiding this comment.
The gist of the PR.
| benchProgramAgdaCek :: String -> Program -> Benchmark | ||
| benchProgramAgdaCek name (UPLC.Program _ _ term) = | ||
| let !term' = force term | ||
| in bench name $ whnf unsafeRunAgdaCek term' |
There was a problem hiding this comment.
Should just call benchTermAgdaCek instead. Will fix.
| benchClausify :: Clausify.StaticFormula -> Benchmarkable | ||
| benchClausify f = nf Clausify.runClausify f | ||
| benchClausify :: String -> Clausify.StaticFormula -> Benchmark | ||
| benchClausify name f = bench name $ whnf Clausify.runClausify f |
There was a problem hiding this comment.
Changed nf to whnf for consistency. Note that this is just the Haskell version of the benchmarks, the CEK one is unaffected by this change.
|
Comparing benchmark results of 'nofib' on 'ee15d369f' (base) and '3ce43665b' (PR) Results table
|
|
Click here to check the status of your benchmark. |
|
Comparing benchmark results of 'validation' on 'ee15d369f' (base) and '3ce43665b' (PR) Results table
|
|
Click here to check the status of your benchmark. |
effectfully
force-pushed
the
effectfully/benchmark/force-outside-of-bench
branch
from
3ce4366 to
76a84bf
Compare
|
Comparing benchmark results of 'validation' on 'ee15d369f' (base) and '3ce43665b' (PR) Results table
|
|
That somehow broke the and I keep seeing it even if I change benchTermCek :: String -> LedgerApi.EvaluationContext -> Term -> Benchmark
benchTermCek name evalCtx term =
let !term' = force term
in bench name $ whnf (evaluateCekForBench evalCtx) term'to benchTermCek :: String -> LedgerApi.EvaluationContext -> Term -> Benchmark
benchTermCek name evalCtx term =
bench name $ let !term' = force term in whnf (evaluateCekForBench evalCtx) term'which really should restore the old behavior? For the love of God, what can possibly be wrong here? |
|
/benchmark bitwise-bench |
1 similar comment
|
/benchmark bitwise-bench |
|
Click here to check the status of your benchmark. |
|
/benchmark nofib |
2 similar comments
|
/benchmark nofib |
|
/benchmark nofib |
|
/benchmark nofib |
|
Click here to check the status of your benchmark. |
|
Click here to check the status of your benchmark. |
|
wtf...
|
|
Comparing benchmark results of 'nofib' on 'ee15d369f' (base) and '76a84bfc5' (PR) Results table
|
|
Click here to check the status of your benchmark. |
|
Comparing benchmark results of 'nofib' on 'ee15d369f' (base) and '76a84bfc5' (PR) Results table
|
|
Click here to check the status of your benchmark. |
|
Comparing benchmark results of 'nofib' on 'ee15d369f' (base) and '76a84bfc5' (PR) Results table
|
|
Click here to check the status of your benchmark. |
|
Comparing benchmark results of 'nofib' on 'ee15d369f' (base) and '76a84bfc5' (PR) Results table
|
|
/benchmark bitwise-bench |
3 similar comments
|
/benchmark bitwise-bench |
|
/benchmark bitwise-bench |
|
/benchmark bitwise-bench |
|
Click here to check the status of your benchmark. |
|
Click here to check the status of your benchmark. |
|
Click here to check the status of your benchmark. |
|
Click here to check the status of your benchmark. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was investigating an issue with #7012 not being faster than the baseline despite clearly doing strictly less work and came to the conclusion that the only reasonable explanation is that forcing of inputs has to happen before
bench. I.e. if we do it after like is currently the case, then it's included in the benchmarking results, even if we do it beforewhnf.