[4.0] Remove global WASM::check_limits and initial memory from instantiate_module #975
Conversation
| @@ -180,7 +162,6 @@ namespace eosio { namespace chain { | |||
| try { | |||
| Serialization::MemoryInputStream stream((const U8*)bytes.data(), | |||
There was a problem hiding this comment.
need to remove all of this section
There was a problem hiding this comment.
oha, thanks! that's the reason of the test failure.
| @@ -177,20 +159,8 @@ namespace eosio { namespace chain { | |||
| std::vector<U8> bytes = { | |||
| (const U8*)codeobject->code.data(), | |||
| (const U8*)codeobject->code.data() + codeobject->code.size()}; | |||
There was a problem hiding this comment.
I think we can get rid of bytes too? could be up to 1MB, on the big side for a needless copy
There was a problem hiding this comment.
You are right. bytes is not needed any more. Removing it saves an unnecessary copy.
|
@heifner my hunch is this solves #964 (comment) because it ought to make WAVM only ever touched on a single thread (...in a process). If it's easy to retest would be interesting to confirm. |
| @@ -174,23 +156,8 @@ namespace eosio { namespace chain { | |||
| }); | |||
| trx_context.pause_billing_timer(); | |||
| IR::Module module; | |||
There was a problem hiding this comment.
one more line here that can get the chop
There was a problem hiding this comment.
Sorry I should be more thorough. Don't know why my build did not show unused variables warning.
Yes, it was rather easy to reproduce, I'll re-test with this PR, merged with |
There was a problem hiding this comment.
Looks good. I've made note in #673 to remove WASM::check_limits entirely at that time
Resolved #969
#969 identified global scoped_skip_checks can be accessible by multiple read-only threads. This variable is not needed in
wasm_interface_impl::get_instantiated_module.Another finding was initial memory argument in
instantiate_modulewas not needed in all theinstantiate_modulemethods. I double checked that it was un-named in all the implementations ofinstantiate_module. Safe to remove.This PR removed both of them.