optimize in for low memory device #2535
Conversation
primstar-cool
commented
Jan 26, 2025
primstar-cool
commented
- add step OnSkipFunctionBodyExpr when read with skip_function_bodies
- add virtual op code OpCodeRaw, when read with skip_function_bodies, the virtual struct will save byte code in a uint8 array, and will write directly on output
- add macro wabt_expr_make_unique, for override operator new
- add MemoryAllocStream can output byte code in an allocated buffer
+ add step OnSkipFunctionBodyExpr when read with skip_function_bodies + add virtual op code OpCodeRaw, when read with skip_function_bodies, the virtual struct will save byte code in a uint8 array, and will write directly on output + add macro wabt_expr_make_unique, for override operator new + add MemoryAllocStream can output byte code in a allocated buffer
There was a problem hiding this comment.
I'm not sure I understand the motivation here. The wabt interpreter (IIUC) is really just designed a reference interpreter. I'm not sure the goal should be to run large program efficiently on resource constrained devices. I think we can leave that the production runtimes out there.
Perhaps you could link to the motivating issue and we could discuss more there?
src/binary-reader-logging.cc
Outdated
| @@ -851,6 +860,8 @@ DEFINE_INDEX_DESC(OnDelegateExpr, "depth"); | |||
| DEFINE0(OnDropExpr) | |||
| DEFINE0(OnElseExpr) | |||
| DEFINE0(OnEndExpr) | |||
| DEFINE_SKIP(OnSkipFunctionBodyExpr) | |||
There was a problem hiding this comment.
I don't think there is any need to define a macro if there is only one caller.
There was a problem hiding this comment.
i will expand the macro in next commit
src/binary-reader.cc
Outdated
| @@ -1939,6 +1940,17 @@ Result BinaryReader::ReadInstructions(Offset end_offset, const char* context) { | |||
| return Result::Error; | |||
| } | |||
|
|
|||
| Result BinaryReader::ReadSkipedFunctionBody(Offset end_offset) { | |||
| std::vector<uint8_t> opcode_buffer; | |||
| opcode_buffer.resize(end_offset - state_.offset); | |||
There was a problem hiding this comment.
Can you just combine there two lines?
std::vector<uint8_t> opcode_buffer(end_offset - state_.offset)
include/wabt/stream.h
Outdated
| @@ -195,6 +195,36 @@ class MemoryStream : public Stream { | |||
| std::unique_ptr<OutputBuffer> buf_; | |||
| }; | |||
|
|
|||
| class MemoryAllocStream : public wabt::Stream { | |||
There was a problem hiding this comment.
I don't see any uses of this new class. Can you perhaps split this out into its own PR along with a description of why its needed?
There was a problem hiding this comment.
sometimes we can't use std mem alloc (such as run in a pre-allocated mempool)
if we use MemoryStream and copy to managed buffer, it will cost double size.
in low-mem device, we need prevent it.
it can be removed from this feature, and impl by proj.
and macro wabt_expr_make_unique is the same reason. we need prevent huge mem allocate(size or times).
final export override for its size, expr override alloc for frequency
There was a problem hiding this comment.
i'v remove it in new commit
+make old skip version
-optimize code on suggestion
| @@ -429,7 +429,8 @@ enum class ExprType { | |||
| Unreachable, | |||
|
|
|||
| First = AtomicLoad, | |||
| Last = Unreachable | |||
| Last = Unreachable, | |||
| OpCodeRaw = Last + 1, // virtual type | |||
There was a problem hiding this comment.
correct us if we're wrong, but you should be able to put it into the enum as normal, between Nop and RefIsNull. you'll need to update the matching array in uh ir.cc (and maybe other places) but the alternative would be to make the code reject OpCodeRaw everywhere else.
also, how about calling this UndecodedOpcodes (between Unary and Unreachable)?
| namespace wabt { | ||
|
|
||
| #ifdef WABT_OVERRIDE_ALLOC_EXPR |
There was a problem hiding this comment.
would you mind limiting this PR to skip_function_bodies for ReadBinaryIr?
| Location& loc, | ||
| BinaryReaderDelegate* delegate, | ||
| const ReadBinaryOptions& options) { | ||
| BinaryReader reader(static_cast<const uint8_t*>(data) - loc.offset, |
There was a problem hiding this comment.
this is undefined behavior. would it be possible to add a "virtual offset" to the BinaryReader instead?
