[webkit-reviews] review granted: [Bug 219297] [WASM-References] Add table.init : [Attachment 415850] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted Dmitry <dbezhetskov at igalia.com>'s
request for review:
Bug 219297: [WASM-References] Add table.init
https://bugs.webkit.org/show_bug.cgi?id=219297

Attachment 415850: Patch

https://bugs.webkit.org/attachment.cgi?id=415850&action=review




--- Comment #16 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 415850
  --> https://bugs.webkit.org/attachment.cgi?id=415850
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=415850&action=review

r=me with one important optimization.

> Source/JavaScriptCore/wasm/WasmInstance.cpp:77
> +    m_passiveElements.resize(m_module->moduleInformation().elementCount());
> +    for (unsigned elementIndex = 0; elementIndex < m_passiveElements.size();
++elementIndex) {
> +	   const auto& element =
m_module->moduleInformation().elements[elementIndex];
> +	   if (element.isPassive())
> +	       m_passiveElements[elementIndex] = element;
> +    }

I think m_passiveElements can be a BitVector, and this is better in terms of
memory.
m_module is held by WasmInstance. And we can get corresponding element by
m_module->moduleInformation().elements[index].
So, only thing we would like to know is that the given passive element is
dropped or not.
Can you change it to save memory? WasmInstance is kept alive so long as wasm
code is running. So saving memory is very important.

> Source/JavaScriptCore/wasm/WasmInstance.h:233
> +    Vector<Optional<Element>> m_passiveElements;

Let's make it BitVector.