perf: for loop and if #2140
perf: for loop and if #2140
Conversation
petar-dambovaliev
requested review from
jaekwon,
moul,
piux2 and
deelawn
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2140 +/- ##
==========================================
+ Coverage 49.01% 49.12% +0.11%
==========================================
Files 576 576
Lines 77604 78288 +684
==========================================
+ Hits 38040 38462 +422
- Misses 36481 36698 +217
- Partials 3083 3128 +45
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| if isUntyped(xt) { | ||
| if t == nil { | ||
| t = defaultTypeOf(xt) | ||
| } | ||
| } |
There was a problem hiding this comment.
This block is useless, t is no longer used afterwards
| @@ -2490,6 +2490,22 @@ func checkOrConvertType(store Store, last BlockNode, x *Expr, t Type, autoNative | |||
| } | |||
| } | |||
|
|
|||
| func checkWithoutConvertType(store Store, last BlockNode, x *Expr, t Type, autoNative bool) { | |||
There was a problem hiding this comment.
I found there is a similar "optimised case" -- checkOrConvertIntegerType. What do you think of changing this function to something like this?
// like checkOrConvertType() but for bools (ie. for and if).
func checkOrConvertBoolType(store Store, last BlockNode, x Expr) {
if cx, ok := x.(*ConstExpr); ok {
convertConst(store, last, cx, BoolType)
} else if x != nil {
xt := evalStaticTypeOf(store, last, x)
if xt.Kind() != BoolKind { /* panic... */ }
}
}I think this might push the performance improvement a bit further :)
| @@ -245,6 +245,38 @@ func main() { | |||
| m.RunMain() | |||
| } | |||
|
|
|||
| func BenchmarkForLoop(b *testing.B) { | |||
There was a problem hiding this comment.
What do you think of having these simply as part of the benchdata tests?
There is a loop.gno test already, there's one missing for loop+if that you added. You can also run them individually:
go test -bench Benchdata/loop -run=^$ ./gnovm/pkg/gnolang/
There was a problem hiding this comment.
Nevermind, I understand now the benchmarks are about preprocessing, not execution. Can you clarify that in the name? Ie. BenchmarkPreprocessForLoop
solves this
Benchmarks
By avoiding the runtime type conversion, we can see that we almost doubled the performance of these statements.