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

Instance.GetFunction Cache #294

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

Conversation

martindevans
Copy link
Contributor

@martindevans martindevans commented Jan 16, 2024

Added a cache into the Instance.GetFunction and Instance.GetMemory methods. This reduces a benchmark fetching a function 1,000,000 times from 300ms to 30ms.

…` methods. This reduces a benchmark fetching a function 1,000,000 times from 300ms to 30ms.
@peterhuene
Copy link
Member

Hi @martindevans, sorry for the delay in reviewing this!

These changes look good, only I'm having a hard time thinking about the real world use case that would necessitate the caching in the API, as I think the typical use case would, at worst, simply store the returns of GetFunction where needed.

I would imagine that it's fairly rare to call GetFunction more than a few times for an instance (at most once per export?) in typical usage and I'm less concerned with implementing a fix to improve a micro benchmark.

Would you mind sharing a usage pattern that you think would directly benefit from having an internal synchronized cache?

@martindevans
Copy link
Contributor Author

The various performance improvement PRs I've been submitting recently are all driven by a game I'm working on at the moment, which allows users to write code (in WASM) to run in game units. There may be around 100 different WASM instances all running in the game, all of which require various calls every frame. The game runs a "sim" as quickly as possible and then saves the replay, so speed is very important!

Initially I was not saving the results of GetFunction, it's easier to simply pass in an Instance to a game system and have it get whatever functions it needs. However profiling revealed that was taking a lot of time (100 ticks/second * 20 minute replay * 100 units * 10 calls per unit == ~30 seconds) over one run of a sim! Obviously I've solved that internally by wrapping the Instance in another class and caching functions, but I thought it'd be better to push that upstream so everyone can get the benefits :)

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