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

feat: use GetVariableAs generic method #523

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TAdev0
Copy link
Member

@TAdev0 TAdev0 commented Jul 11, 2024

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)

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.

GetVariableAs methods in Scope Create a structure for Scope value types instead of using interface
1 participant