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: uses memoization to decrease memory consumption. #13

Merged
merged 1 commit into from
Nov 20, 2023

Conversation

jcchavezs
Copy link
Member

No description provided.

}

// makeDoer returns a function that executes and returns the results of the given function
func makeDoer(cache *sync.Map) func(string, func() (interface{}, error)) (interface{}, error, bool) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I know I suggested copying the same PR. But realized that since this is wasilibs we shouldn't use any of these sync mechanisms - there are no threads (maybe in the future but I'd rather deal with it when it happens). So it'd be good to simplify the implementation and tests to just use a normal map and no other libraries.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same thought about the sync package but then realise people could
still want to use wasilibs with go, and hence the sync isn't? Still we can
remove it for now until someone requests for it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's probably fine. I have bad memories of a sync pool not actually pooling for some time in TinyGo and have some caution towards that package but if sync.Map is mostly a normal map in practice it should be ok

@jcchavezs jcchavezs merged commit 9688ecb into main Nov 20, 2023
2 checks passed
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.

2 participants