Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GopherJS generics support #1013

Open
7 tasks
nevkontakte opened this issue Apr 5, 2021 · 25 comments
Open
7 tasks

GopherJS generics support #1013

nevkontakte opened this issue Apr 5, 2021 · 25 comments
Labels
enhancement NeedsHelp Community contributions are welcome for this feature!

Comments

@nevkontakte
Copy link
Member

nevkontakte commented Apr 5, 2021

It appears that generic (a.k.a. type parameters support) is well underway in the upstream Go (dev.typeparams branch) and is likely to be included in 1.18. Which means GopherJS also needs to support this, ideally at the same time as Go 1.18, which I expect is a lot of work. Does anyone have even approximate sense of what it would take to support generics? @flimzy @dmitshur @neelance.

For now, I'm filing this issue to look for a volunteer (or several) to contribute this support. By myself I almost certainly won't be able to implement this, considering that we have several other important features from earlier releases to take care of, such as modules and embed.

Important TODOs:

  • Re-enable encoding/xml tests once generics are supported.
  • Re-enable checkStringParseRoundTrip() in net/netip/fuzz_test.go.
  • Remove compareSlices() override in go/doc/doc_test.go.
  • Re-enable TestIssue50208 in reflect package.
  • Verify pointers to arrays are wrapped correctly in generic functions.
  • [go1.19] Remove generics-related overrides in crypto/elliptic/nistec.go, crypto/internal/boring/bbig/big.go, crypto/internal/nistec/nistec_test.go, crypto/internal/nistec/wrapper.go, go/token/position.go, sync/atomic/atomic.go, sync/atomic/atomic_test.go, testing/helper_test.go, testing/helperfuncs_test.go
  • [go1.20] Remove generics-related overrides in compiler/natives/src/sync/map.go, compiler/natives/src/time/format.go, compiler/natives/src/time/format_rfc3339.go, compiler/natives/src/encoding/gob/gob.go, compiler/natives/src/internal/godebug/godebug.go, compiler/natives/src/net/http/http.go, compiler/natives/src/time/export_test.go
@nevkontakte nevkontakte added enhancement NeedsHelp Community contributions are welcome for this feature! labels Apr 5, 2021
@inliquid
Copy link

inliquid commented Apr 9, 2021

I think it's planned for go1.18.

@nevkontakte nevkontakte pinned this issue Dec 19, 2021
@nevkontakte nevkontakte mentioned this issue Jul 31, 2022
5 tasks
nevkontakte added a commit to nevkontakte/gopherjs that referenced this issue Jul 31, 2022
One of the helper functions requires generics to work correctly. Until
they are properly supported we have to stub it out.

Generics support is tracked in
gopherjs#1013.
nevkontakte added a commit to nevkontakte/gopherjs that referenced this issue Jul 31, 2022
One of the helper functions requires generics to work correctly. Until
they are properly supported we have to stub it out.

Generics support is tracked in
gopherjs#1013.
nevkontakte added a commit to nevkontakte/gopherjs that referenced this issue Jul 31, 2022
One of the helper functions requires generics to work correctly. Until
they are properly supported we have to stub it out.

Generics support is tracked in
gopherjs#1013.
nevkontakte added a commit to nevkontakte/gopherjs that referenced this issue Jul 31, 2022
Even though the compiler was usually able to compile them into
_something_, the code was likely incorrect in all cases. To prevent
users from tripping up on that the compiler will return an error if it
encounters type params until
gopherjs#1013 is resolved.
nevkontakte added a commit to nevkontakte/gopherjs that referenced this issue Jul 31, 2022
Even though the compiler was usually able to compile them into
_something_, the code was likely incorrect in all cases. To prevent
users from tripping up on that the compiler will return an error if it
encounters type params until
gopherjs#1013 is resolved.
nevkontakte added a commit to nevkontakte/gopherjs that referenced this issue Aug 1, 2022
One of the helper functions requires generics to work correctly. Until
they are properly supported we have to stub it out.

Generics support is tracked in
gopherjs#1013.
nevkontakte added a commit to nevkontakte/gopherjs that referenced this issue Aug 1, 2022
Even though the compiler was usually able to compile them into
_something_, the code was likely incorrect in all cases. To prevent
users from tripping up on that the compiler will return an error if it
encounters type params until
gopherjs#1013 is resolved.
nevkontakte added a commit to nevkontakte/gopherjs that referenced this issue Aug 1, 2022
Even though the compiler was usually able to compile them into
_something_, the code was likely incorrect in all cases. To prevent
users from tripping up on that the compiler will return an error if it
encounters type params until
gopherjs#1013 is resolved.
@nevkontakte
Copy link
Member Author

I've been doing some research on potential ways to support generics in gopherjs and I think it would be good for me to share my thoughts. This isn't a proper design document, but I hope it would help us start a discussion and perhaps make it easier for people to contribute towards this :)

I'll start by sharing links to a few important resources I used:

These design docs deserve the most attention here, as they describe options the Go team considered for their own implementation. The first two docs present two ends of the spectrum: generate a specialized copy of executable code for each instantiation and generate one copy of universal code, delegating type-specific logic to helper functions passed via the dictionary. Ultimately, the Go team went with the hybrid approach as a trade off between binary size, compile time and complexity.

I think for GopherJS the tradeoff looks significantly differently. For us, output size is a critical concern, but things like stack allocation or memory management are much less relevant, since we delegate those to the javascript runtime. Also on a technical level, stenciling requires the ability to add function instantiations to the imported packages and deduplicate them at the linking stage. This is solvable, but GopherJS is not very well set up for this and will require a significant refactoring to make it possible. This makes me strongly favor the dictionaly-based design.

However, we can take advantage of our dynamic runtime environment and simplify our implementation even further by delaying generics instantiation to the runtime. That way, the compiler won't have to generate dictionaries and subdictionaries from the original design, or include them into the generated binary.

Consider this non-generic type:

type Foo struct{ f string }
func (f Foo) Bar() { println("bar") }

Here is a slightly abbreviated version of the code it compiles into:

Foo = $newType(0, $kindStruct, "main.Foo", true, "main", true, function(f_) { /* field initialization code */ });
Foo.ptr.prototype.Bar = function() { /* Foo.Bar() code */ };
Foo.methods = [{prop: "Bar", name: "Bar", pkg: "", typ: $funcType([], [], false)}];
Foo.init("main", [{prop: "f", name: "f", embedded: false, exported: false, typ: $String, tag: ""}]);

main = function() {
  var f = new Foo.ptr(""); // Using the type!
  $clone(f, Foo).Bar();
};

The important insight here is that we are already delaying type instantiation to the runtime, just before we begin program execution. Why not do the same for generic types, except do this when the generic type is demanded by the program?

Consider the following generic type:

type Foo[T any] struct{ f T }

func (f Foo[T]) Bar() { println("bar") }

Instead of a concrete instantiation we could generate a factory function for it:

Foo = function(T) {
  var instance = $newType(0, $kindStruct, "main.Foo[" + T.typeString +"]", true, "main", true, function(f_) { /* field initialization code */ });
  // Note that the method body closes over the type T and can use it for reflection/whatever purposes.
  instance.ptr.prototype.Bar = function() { /* Foo.Bar() code */ };
  instance.methods = [{prop: "Bar", name: "Bar", pkg: "", typ: $funcType([], [], false)}];
  // Note "typ: T" on the next line — using type parameter!
  instance.init("main", [{prop: "f", name: "f", embedded: false, exported: false, typ: T, tag: ""}]);
  return instance;
}

main = function() {
  var f = new (Foo($String).ptr)(""); // Calling the factory to get the constructor for the new operator.
  $clone(f, Foo($String)).Bar();
};

Of course, this is a simplified example. In the real implementation the factory function will cache and reuse instances of a generic type to improve performance and avoid "same but different" kinds of type conflicts. Similar approach could be applied to standalone generic functions.

Another issue we need to address is type-specific operations. The same operator may mean different things for different types, for example:

  • int32(1) + int32(2) compiles into 1 + 2 >> 0
  • "one" + "two" compiles into "one" + "two"

The original design solves this by introducing dictionaries that have a virtual table of functions that can be used with the generic type. In our implementation we can use the type instance itself in that role. For example we could define $String.add() and $Int32.add(), which could be called in the generic code as T.add(). Admittedly, this is likely to be worse performance that using the operator directly, but we could modify the compiler to only use these functions in the generic code, thus preventing performance hit in the existing non-generic code.

Finally, we also need to be able to construct types in generic code from passed type parameters, for example:

func Baz[T any]() { 
  x = Foo[[]T]{}
  //...
}

That could be compiled into something like this:

Baz = function(T) {
  return function() {
    x = new (Foo($silceType(T)))();
    // ...
  }
}

So that's as far as I got. It's getting late here, so apologies for all the typos I'm sure I've made in this text 😅 Inviting @flimzy, @paralin and any one else interested to comment.

@nevkontakte
Copy link
Member Author

I created the new generics branch for this work and enabled the typeparam test suite from the Go repository in it. This should give us a decent idea for the level of compatibility we currently have.

@paralin are you still planning to work on it? I think I might start chipping away on this issue, and it would be a shame if we did duplicate work :) That said, I think there is plenty enough work for two people, if you are willing to help.

@paralin
Copy link
Contributor

paralin commented Sep 18, 2022

@nevkontakte I haven't started looking into this yet, but I'm happy to help out where I can if you have something you need done.

@nevkontakte
Copy link
Member Author

Since I haven't really started coding yet, I can't point at anything specific 😅 My general plan is to look at CI, pick a failing test case and try to make it pass, following the general approach I posted in the earlier comment. I might also use you as a reviewer for my own changes, hopefully that will give you ideas what to do next.

@paralin
Copy link
Contributor

paralin commented Sep 21, 2022

@nevkontakte Sounds good! I haven't had any time to look at this yet

nevkontakte added a commit to nevkontakte/gopherjs that referenced this issue Oct 15, 2022
Instead of generating an independent function instance for every
combination of type parameters at compile time we construct generic
function instances at runtime using "generic factory functions". Such a
factory takes type params as arguments and returns a concrete instance
of the function for the given type params (type param values are
captured by the returned function as a closure and can be used as
necessary).

Here is an abbreviated example of how a generic function is compiled and
called:

```
// Go:
func F[T any](t T) {}
f(1)

// JS:
F = function(T){ return function(t) {}; };
F($Int)(1);
```

This approach minimizes the size of the generated JS source, which is
critical for the client-side use case, at the cost of runtime
performance. See gopherjs#1013 (comment)
for the detailed description.

Note that the implementation in this commit is far from complete:

  - Generic function instances are not cached.
  - Generic types are not supported.
  - Declaring types dependent on type parameters doesn't work correctly.
  - Operators (such as `+`) do not work correctly with generic
    arguments.
nevkontakte added a commit to nevkontakte/gopherjs that referenced this issue Oct 15, 2022
Instead of generating an independent function instance for every
combination of type parameters at compile time we construct generic
function instances at runtime using "generic factory functions". Such a
factory takes type params as arguments and returns a concrete instance
of the function for the given type params (type param values are
captured by the returned function as a closure and can be used as
necessary).

Here is an abbreviated example of how a generic function is compiled and
called:

```
// Go:
func F[T any](t T) {}
f(1)

// JS:
F = function(T){ return function(t) {}; };
F($Int)(1);
```

This approach minimizes the size of the generated JS source, which is
critical for the client-side use case, at the cost of runtime
performance. See gopherjs#1013 (comment)
for the detailed description.

Note that the implementation in this commit is far from complete:

  - Generic function instances are not cached.
  - Generic types are not supported.
  - Declaring types dependent on type parameters doesn't work correctly.
  - Operators (such as `+`) do not work correctly with generic
    arguments.
@nevkontakte
Copy link
Member Author

nevkontakte commented Nov 2, 2022

I am now beginning to second-guess the wisdom of the approach I've described above...

On one hand it does the benefit of the smallest possible output size, which is important. On the other hand, it will make generics slower than normal function calls in numerous fundamental ways. Here is an list of performance issues I have discovered so far (there probably are more):

  • Calling a generic function will be two calls: first to a the generic function factory, then to the function itself.
  • When applied to generic types, operators like + will have to become virtual function calls because different primitive types implement them differently.
  • "Wrapped" types (e.g. types that gopherjs represents as JS native types at runtime, only wrapping in Go-specific object when necessary, such as type x int) will have to be wrapped in the function prologue, which is a memory allocation.
  • Each method call for a typeparameter-dependent object would have to be considered as a potentially blocking, with the corresponding overhead (like it is with interface method calls).

That said, going full monomorphization is also not easy from the compiler architecture perspective, and will cause obvious output size blowups.

@paralin
Copy link
Contributor

paralin commented Nov 2, 2022

@nevkontakte I think it's worthwhile to consider that most users will end up running the code through a js optimizer at some point. So potentially two things: maybe code size isn't as important? Or possibly, are the optimizers capable of reducing the multiple calls to fewer, so it doesn't matter? Not sure which is true

@nevkontakte
Copy link
Member Author

@paralin in our user survey output file size is by far the biggest pain point:
image

So it's hard for me to argue that it isn't that important.

In general it's kind of hard to imagine how a generic JS optimizer can help with deduplicating monomorphized function bodies, since the will have to be semantically different from each other, even if similar. For multiple function calls my best bet is on the JIT compiler JS engines may have, but those work in mysterious ways and it's hard to tell would would be inlined away and what wouldn't.

A part of me thinks that it all would be way easier if GopherJS used SSA representation to generate the code, but that's a huge undertaking in itself, and there is a non-trivial chance that SSA would end up unsuitable in the end (particularly, transpiling SSA into a high-level language might lead to a more verbose and less readable JS code than we have today).

@paralin
Copy link
Contributor

paralin commented Nov 2, 2022

@nevkontakte I see what you're saying. Yeah, that was what I was wondering - how much do these JS optimizers actually restructure the code to improve performance... Is it like GCC where there are structural optimizations going on, or is it just a simple uglify type pass?

From some research I guess today the "optimizers" of the JS world are very much in an early stage only: https://prepack.io/

So indeed it's important to optimize for these things at the code-gen time and not lean on post-processing.

Apologies for going down this side-road in this discussion!

@nevkontakte
Copy link
Member Author

I guess now would be a good time for a small confession... The longer I've been working on a generics implementation I described here, the less happy I was with it.

There were two and a half major problems with it:

  • The biggest one is that generics became a gigantic special case in the compiler. A lot of type-specific stuff that the compiler would normally be able to handle for non-generic code had to be replicated in the runtime for generics. The half-reason is that this meant prelude was getting bigger and more complex -> the output size grew, even if generics were not be used.
  • Generics would would be unavoidably slower than non-generic one, which is both sad and counter-intuitive. To illustrate why, consider integer addition: if we know we are working with uint8 we can generate something like (x+y)%256 , which is quite cheap; however, if we are working with generics, it would have to be T.$add(x, y), where T is a runtime reference to uint8 type. The latter is considerably more expensive because it now involves a dynamic method call. We can hope that JIT will eventually inline it, but that's not a given.

So, for the last couple of months I've been working on an alternate approach, which is now #1272. In the good news, it's passing nearly all generic-related test cases from the Go repo. However, there is a bit more work before we can merge that implementation. So, stay tuned.

@nevkontakte
Copy link
Member Author

@flimzy how do you feel about merging the generics-ng branch into master in its current state, gating generics support behind some sort of GOPHERJS_EXPERIMENT flag? Generics are not production-ready yet, but I think it's very unlikely we'd take any major direction changes in the implementation. I am currently looking into wiring cross-package generic instances and it will likely require some non-trivial refactoring on the build package and tool.go side to do it cleanly. Doing that refactoring in an isolated branch is likely to turn into a merge conflict headache, especially with the Go 1.20 work @grantnelson-wf is doing. Moving most of the generic development into master would make it easier to keep 1.20 work up to date with it.

@flimzy
Copy link
Member

flimzy commented Mar 16, 2024

Sounds reasonable to merge given the rationale above. +1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement NeedsHelp Community contributions are welcome for this feature!
Projects
None yet
Development

No branches or pull requests

5 participants