Skip to content
This repository was archived by the owner on Apr 25, 2025. It is now read-only.

Allow immutable fields in struct.new_default_with_rtt #174

Closed
manoskouk opened this issue Jan 11, 2021 · 15 comments · Fixed by #336
Closed

Allow immutable fields in struct.new_default_with_rtt #174

manoskouk opened this issue Jan 11, 2021 · 15 comments · Fixed by #336

Comments

@manoskouk
Copy link
Contributor

It would be useful to be able to initialize structs with default fields even if there are immutable fields. In that case, the immutable fields would need to be provided as arguments to struct.new_default_with_rtt.

As a use case, consider a Java object: it will always have an immutable field pointing to the virtual method table, but it also needs to have its fields initialized to their default value at allocation (as is dictated by Java semantics).

@Horcrux7
Copy link

Yes, this can be helpful for my Java implementation. It can be reduce a lot of code.

copybara-service bot pushed a commit to google/j2cl that referenced this issue Jan 28, 2021
… on instance initialization to allow the assignment of the vtable.

struct.new_default_with_rtt does not allow to initialize immutable fields to values other than null. In our model the vtable (or type information block) needs to be immutable to satisfy the wasm typing model.

There is a proposal to allow struct.new_default_with_rtt receive the values for all the immutable fields:
  WebAssembly/gc#174

PiperOrigin-RevId: 354404824
@RossTate
Copy link
Contributor

RossTate commented Feb 2, 2021

If code size is the primary concern here, we might want to consider explicit inlining here, such as @kripken's suggestion in WebAssembly/design#1375 (comment). The reason is that inlining also addresses a related classic problem in modules at separate compilation. If you consider a Java class in some separately compiled module, it cannot inline the initialization code for its super classes (including Object) because, per separate compilation, it does not know what that code is and may not even have direct access to the relevant fields as they've been encapsulated as private. So every allocation of your own class would require a function call, which historically has had notable effect on performance (though unfortunately I can't remember the name of the paper I've seen on this). Explicit inlining would provide a very simple macro here (while keeping the instruction set simple) and improve performance for separate compilation where inlining cannot be done manually.

@rossberg
Copy link
Member

rossberg commented Feb 3, 2021

I see how this can be useful, though my gut reaction would be to consider this a post-MVP addition, since it is a local optimisation that doesn't add crucial functionality. And it is not immediately obvious how to derive the selection of non-defaulted fields (all immutables plus all non-defaults? or something more customisable? either way, it would be a rather complex instruction).

@Horcrux7
Copy link

Horcrux7 commented Feb 3, 2021

In my Java compiler it would only relevant for the vtable currently which is an int32.

Instead a simple:

    i32.const vtableIdx
    struct.new_default_with_rtt $t

I need the follow code:

    struct.new_default_with_rtt $t
    local.tee $extra_local
    local.get $extra_local
    i32.const vtableIdx
    struct.set $t 0

This are 3 extra operation and an extra local for every struct allocation and the vtable is not immutable.

I have also evaluate to push first all default values. But with a medium of 10 fields per structs the overhead is much larger.

@rossberg
Copy link
Member

rossberg commented Feb 4, 2021

Right, but keep in mind that Wasm is an assembly language, so this is to be expected. Wasm code mostly mirrors what native code would have to do as well. We have so far resisted adding "macro instructions" to Wasm, unless they have common counterparts in actual hardware or cannot be avoided for other reasons. (Arguably, the existing new_default line of instructions is already violating that philosophy, so should perhaps even be removed.)

@RossTate
Copy link
Contributor

RossTate commented Feb 4, 2021

@Horcrux7 In the MVP doc, it suggests that v-tables be implemented using a struct, but you're using a numeric index. Could you give some background as to why?

@Horcrux7
Copy link

Horcrux7 commented Feb 4, 2021

There are multiple reasons why I use an i32 currently.

  • The compiler has the option to work also without GC and JavaScript glue code instead. With this option it can run in the current browsers. For this mode I need a vtable without structs.
  • a struct produce a large overhead. My vtable is not only a vtable. It is also an itable and meta data for reflection like the class name. Most of the data will never use. Copy all the data to a struct produce a unneeded overhead.
  • With a single wasm file I see no advantages. The vtables are saved in the data section currently.
  • also the call.ref was not available at development time.

But I will evaluate the performance of a vtable with a struct later. It will be interesting.

@Horcrux7
Copy link

Horcrux7 commented Feb 4, 2021

Right, but keep in mind that Wasm is an assembly language

@rossberg Wasm is extremely limit comparing to a assembly language. For example In assembly I can fill allocated memory of a struct with zero bytes.

@RossTate
Copy link
Contributor

RossTate commented Feb 4, 2021

That all makes sense. My concern is that using an i32 seems likely to break down when we eventually get to supporting separate compilation. But if you plan on evaluating performance with a v-table as well, that'll be really informative. I'm worried that (what I believe is the current implementation of) function references will cause traditional v-tables to perform poorly compared to how they could, and your planned comparison should hopefully be able to identify that gap (if it's there).

@tlively
Copy link
Member

tlively commented Nov 1, 2022

@manoskouk, do you think this is still worth discussing for inclusion in the MVP or should we close this issue? We can also add it as a post-MVP feature idea in the post-MVP doc.

@manoskouk
Copy link
Contributor Author

We can definitely postpone this to post-MVP.

@oovm
Copy link
Contributor

oovm commented Mar 6, 2024

Even defaultable types sometimes want to be initialized to a non-zero value

For example, the following statement in C#:

class CustomDefault
{
    public int Field = 42;
}

So can we declare the default typed value when struct type is declared?

@rossberg
Copy link
Member

rossberg commented Mar 6, 2024

@oovm, your compiler will have to generate that initialisation code, like any regular compiler targeting hardware. Defaults are not intended as a convenience mechanism to replace such code. The only reason they exist is to guarantee a well-defined semantics for Wasm (since it isn't always possible to check initialisation efficiently).

@oovm
Copy link
Contributor

oovm commented Mar 7, 2024

I understand, so is it possible to have instructions with semantics similar to the following:

i32.const 1       ;; overrride defaultable
table.idx funcref ;; fill undefaultable
struct.new_default_with_field $type $field1 $field2 ;; fill other unindexed defaultable types

Under readonly semantics, I only want certain fields to be filled in the constructor.

If I use (mut field) to default initialize it first and then modify it, it cannot reflect the readonly semantics.

@rossberg
Copy link
Member

rossberg commented Mar 7, 2024

Well, sort of, by using struct.new and providing the defaults explicitly.

Also, I'm not sure what you mean by readonly semantics. The Wasm data representation used by a compiler does not need to reflect read/write constraints of the source language (if you were to use linear memory, everything would be mutable as well). The only reason Wasm has immutable fields at all is that they permit more subtyping. But unless the source language you're compiling allows covariant subtyping on these fields, your compiler can simply define them as mutable internally.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants