NethermindEth / cairo-vm-go

A virtual machine for Cairo written in Go
MIT License
79 stars 49 forks source link

feat: use `GetVariableAs` generic method #523

Closed TAdev0 closed 1 month ago

TAdev0 commented 1 month ago

Resolves #448 Resolves #347

@cicr99 @har777 @rodrigo-pino here is a PR for the Scope optimization issue. We ended up saying using a struct for scope values is not a good idea because the struct would contain too many fields. An external contributor finally proposed to simply refactor all the GetVariableAs... method using a generic method GetVariableAs.

I replaced all occurences in the whole codebase.

Did some benchmark to test the generic vs specific function for Big.int :

tristan@Tristans-MBP go test % go test -bench=.
goos: darwin
goarch: arm64
pkg: testGenerics
BenchmarkSpecific-8     330242318                3.400 ns/op
BenchmarkGeneric-8      353439238                3.407 ns/op
PASS

cost of using generics is really really negligible, while it reduces the codebase and makes it cleaner.

What do you think? If we want to optimize the VM the most, i guess we should leave it as it is now, or just refactor a bit to make it more consistent (use a GetVariableValueAs... helper everywhere its needed, or nowhere, not just sometimes)