feat: optimize the active init operation of zero-initialized memory of makeFieldInitializationInConstructor
#2948
Conversation
KKiiim
changed the title
makeFieldInitializationInConstructormakeFieldInitializationInConstructor
|
@CountBleck Could you please have a look on this PR (^_^) |
There was a problem hiding this comment.
This seems mostly okay, but I'm wondering what all the removed $~lib/arraybuffer/ArrayBufferView#set:buffer is about...what is that setter function supposed to be doing anyway?
I'd like @MaxGraey to sign off before merging.
| if (this.currentType != Type.void) { // in case | ||
| expr = module.drop(expr); | ||
|
|
||
| if (initializerNode) { |
There was a problem hiding this comment.
If there isn't an initializerNode, this code neglects to zero-initialize the field when stub runtime is used (and the stub runtime doesn't zero-initialize allocated memory!). The previous code's this.makeZero(fieldType) happened to cover this case.
There was a problem hiding this comment.
Looks no need to zero-initialize because:
- memory will not be collected and reused in stub runtime.
- memory grow will default to initialized with 0 as wasm spec.
There was a problem hiding this comment.
The exception is when the latest allocation is being freed. There's also the case when memory is imported (which means it's not necessarily zeroed), as well as the very niche case when the heap is reset (which also leaves the memory dirty upon future allocations).
You are correct in saying that memory.grow zeroes out the new memory, but the above cases still show why fields need to be zeroed when using the stub allocator.
There was a problem hiding this comment.
Okay, I understand your point.
Perhaps the safest approach here is to simply disable this optimization in stub rt.
Reduce unnecessary zero-initialization memory in the constructor
According to wasm-spec, grown memories should initialized with value 0x00
I've read the contributing guidelines
I've added my name and email to the NOTICE file