Skip to content

Conversation

@ben-page
Copy link

@ben-page ben-page commented Feb 15, 2022

  • The memory allocated for the initial GetData wasn't reused if different data was loaded.
  • FastEvaluate() is called when _dataHeapPtr is near the end of the memory store, it can overflow the store. I created a method to check the memory capacity and grow as necessary.

@christophwille
Copy link
Owner

https://github.com/open-policy-agent/npm-opa-wasm/blob/3e618c84a07ba3af8d7bab75eb77395bd79d7c54/src/opa.js#L267

this library mimics what npm-opa-wasm does in terms of execution order.

@ben-page
Copy link
Author

https://github.com/open-policy-agent/npm-opa-wasm/blob/3e618c84a07ba3af8d7bab75eb77395bd79d7c54/src/opa.js#L267

this library mimics what npm-opa-wasm does in terms of execution order.

I can take out the first commit, if that helps. It's a small amount of wasted memory. The second commit is the important one.

@christophwille
Copy link
Owner

christophwille commented Feb 15, 2022

Yes (and I just saw that npm-opa-wasm has applied memory fixes recently, so I'll have to take a look at their recent history too)

Edit: opened #38 for mem fix note above.

@christophwille
Copy link
Owner

Can we somehow unit test the behavior?

@ben-page
Copy link
Author

Can we somehow unit test the behavior?

I think so. Give a me a couple days.

@christophwille
Copy link
Owner

open-policy-agent/npm-opa-wasm@f1be838 corresponding JS impl

@christophwille
Copy link
Owner

@ben-page I started a new PR #58 but that doesn't work yet - it is basically your code from this PR plus a few amenities and tests - and the tests actually fail. Did your solution work for you in prod?

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