1833614 - Restructuring of wasm-gc allocation methods in preparation for work on inline allocation

Status: RESOLVED DUPLICATE of bug 1839598

(Core :: JavaScript: WebAssembly, enhancement)

Description

[Julian Seward [:jseward]]

As preliminary work for implementing inline wasm-gc object allocation, this
bug contains restructuring to make it easier to see what needs to be
implemented inline. Currently there are four patches:

• one to move WasmStructObject::createStruct from WasmGcObject.h to
  WasmGcObject.cpp, since that's where all the other allocation methods are.
  This gives a small performance loss.

• one that removes WasmGcObject::create, and manually inlines and specialises
  it into its callers.

• one that renames various names of the form StructNew to StructNewInit,
  so as to be consistent with existing names StructNewUninit, and so as to
  reduce confusion in the next patch. Ditto for ArrayNew.

• one that splits the StructNewInit and StructNewUninit methods into two
  variants each, hence: StructNewInitIL, StructNewInitOOL,
  StructNewUninitIL, StructNewUninitOOL. The IL/OOL variants produce
  objects with inline-only storage or outline-storage respectively. They
  remove the existing check at run time about whether this is necessary,
  because that's known at compile time.

This results in new function StructNewUninitIL (uninit, inline only) being
considerably simpler, and something we could consider writing macroassembler
methods for, in the style of existing (for JS)
MacroAssembler::allocateObject. Possibly also for StructNewInitIL.

Even if we don't do that they are a nice cleanup, and more or less
performance-neutral.

Comment 1

[Julian Seward [:jseward]]

Attachment: bug1833614-1-reoutline-createStruct.diff-1

Comment 2

[Julian Seward [:jseward]]

Attachment: bug1833614-2-inline-GCO-create.diff-1

Comment 3

[Julian Seward [:jseward]]

Attachment: bug1833614-3-rename-Init-methods.diff-3

Comment 4

[Julian Seward [:jseward]]

Attachment: bug1833614-4-direct-to-IL-OOL.diff-1

Comment 5

[Julian Seward [:jseward]]

Attachment: bug1833614-5-TypeDefData-structTotalBytes.diff-1

Here's another refinement. It caches the required structure size in
struct TypeDefData, so that the allocator doesn't need to chase from
TypeDefData to TypeDef to find the size. Hence removes a dependent
load in the allocation path, but more importantly removes the need
for allocation of structs to look at the TypeDef at all, which seems
like a useful simplification in advance of inline allocation.

Comment 6

[Julian Seward [:jseward]]

We could really use a sixth patch here, which ensures that the
inline-storage size of a WasmStructObject is always an integral
number of machine words; currently it isn't, for some strange reason.
This would somewhat improve/simplify the zeroing-out code
for that area required by an inline implementation of
WasmStructObject::createStructIL.

Comment 7

[Julian Seward [:jseward]]

Attachment: bug1833614-6-size-integral-words.diff-1

Patch as per comment 6. This seems to marginally improve performance
on Barista-3, possibly by reducing the amount of branch misprediction
induced by memset-zero-ing the inline storage areas.

Comment 8

[Julian Seward [:jseward]]

Closing this bug in favour of bug 1839598, which supersedes it.