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

wasm: improve memory management strategy #1121

Closed
tsandall opened this issue Dec 19, 2018 · 3 comments · Fixed by #1803
Closed

wasm: improve memory management strategy #1121

tsandall opened this issue Dec 19, 2018 · 3 comments · Fixed by #1803
Labels

Comments

@tsandall
Copy link
Member

The current malloc implementation in the WASM-C library grows the heap by the required page count each time it's called (which is expensive and inefficient.) Rough benchmarks show that the latency is dominated by the heap operations. For the initial implementation, it may be possible to allocate the heap once.

@t83714
Copy link
Contributor

t83714 commented Jul 6, 2019

@tsandall The opa_malloc function seems to allocate one page (64KB) even for a Boolean value.

Could this make it a more urgent issue as it seems a memory consumption issue rather than only a latency issue?

@tsandall
Copy link
Member Author

tsandall commented Jul 6, 2019

@t83714 yes this a blocker to using the wasm support in non-trivial cases. We'll hopefully find time to work on this in the next month or so.

@t83714
Copy link
Contributor

t83714 commented Jul 7, 2019

@tsandall Thanks a lot for the update 👍

I was trying to figure out something similar to this for scala to make wasm policies run on the JVM.

As I had to use a fixed size java ByteBuffer as the imported memory, it's quite easy for me to hit the out of memory error.

I actually was not 100% sure whether it's the issue of the scala wasm runtime I used or the issue of OPA memory allocation --- not very familiar with the C code 😄

Glad to know it's been covered by your plan already 👍

tsandall added a commit to tsandall/opa that referenced this issue Sep 17, 2019
The memory.grow calls were dominating the evaluation time and the
integration tests were beginning to take way too long to run. This
just updates the opa_malloc placeholder to keep track of the heap and
only call memory.grow when needed. We can revisit memory management in
the future but for now this is good enough.

Fixes open-policy-agent#1121

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
tsandall added a commit to tsandall/opa that referenced this issue Sep 28, 2019
The memory.grow calls were dominating the evaluation time and the
integration tests were beginning to take way too long to run. This
just updates the opa_malloc placeholder to keep track of the heap and
only call memory.grow when needed. We can revisit memory management in
the future but for now this is good enough. The opa_object and opa_set
functions had to be fixed to zero out their members so that the same
memory could be re-used across multiple evals.

Fixes open-policy-agent#1121

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
tsandall added a commit that referenced this issue Oct 1, 2019
The memory.grow calls were dominating the evaluation time and the
integration tests were beginning to take way too long to run. This
just updates the opa_malloc placeholder to keep track of the heap and
only call memory.grow when needed. We can revisit memory management in
the future but for now this is good enough. The opa_object and opa_set
functions had to be fixed to zero out their members so that the same
memory could be re-used across multiple evals.

Fixes #1121

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
2 participants