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

Applying shared structs to existing code is difficult with no member functions #17

Open
kbabbitt opened this issue Dec 5, 2022 · 1 comment

Comments

@kbabbitt
Copy link
Collaborator

kbabbitt commented Dec 5, 2022

Babylon.js has many existing classes representing primitives such as vectors and colors, with member function operations such as scaleToRef (vector_b = vector_a * scalar) and addInPlace (vector_a += vector_b). I experimented a bit with applying shared structs to these primitives. Since shared structs don't support attaching any sort of code, I had to work around the existence of these operations and found none of the possible solutions particularly satisfying:

  1. Convert the mutator operations to static. This would of course require updating all call sites, and for heavily used classes like vectors and colors, there would be many. It also sacrifices type safety: where before we knew that this was, for example, a color, with a static method we have no such guarantee. This is especially unfortunate given that the origin trial does not yet have language-level support for shared structs, which means Typescript can't even do static checking at compile time.
  2. Instead of converting the color class itself into a shared struct, keep it as a non-shared object but use a shared struct as backing storage under the hood. This works well for a given class in isolation but falls apart when objects are composed together. For example a Particle object might have several colors and vectors as members. In order to process that Particle on a worker thread, we would need some way to extract and pass over a graph of shared structs that underpin a corresponding graph of non-shared objects. That implies a degree of classes having familiarity with each others' internals that I found uncomfortable.
  3. Introduce a means of serializing non-shared classes into shared structs and deserializing them on a worker thread. That may or may not be faster than passing non-shared objects via postMessage or flattening to SharedArrayBuffer, but it certainly isn't going to realize the full potential of sharing across threads.

My intuition after this exercise is that in order for shared structs to apply cleanly to existing codebases, we'll need some way of attaching functions to them. I recognize that there are challenges to doing this, and for what it's worth I think it would be okay if a function attached to a shared struct doesn't have the full set of capabilities that a function attached to a non-shared object has. It's already the case that shared structs are limited - they are of fixed shape - and having limited-capability member functions would follow that precedent. I think the only piece that's needed would be for a member function on a shared struct to be treated as a static function with an implicit this parameter.

@kbabbitt
Copy link
Collaborator Author

Some code samples to help illustrate the changes needed and their impact.

The Babylon.js team pointed me to a particle system demo, located at https://playground.babylonjs.com/#N7PGG3#2, to use as a testbed.
Duplicate of the demo to act as a baseline for comparison: Hosted version, source code.
Modifications for use with a locally hosted build of Babylon: Hosted version, source code.

I made two sets of prototype changes to Babylon:

1. Converting mutator operations to static

https://github.com/kbabbitt/Babylon.js/tree/shared-structs-1-static-mutators

In this branch I've created shared struct representations of Vector2 and Vector3, accessible via the MakeNewShared static on both classes. The Particle object uses these for 3 members: position, direction, and scale. This necessitated updating mutations not just in Babylon itself but also in the demo; see https://github.com/kbabbitt/kbabbitt.github.io/compare/shared-structs-1-static-mutators
There's not a huge number of changes, but hopefully it's enough to show the tip of the iceberg. VS Code informs me there are 463 references to Vector2 and 3574 references to Vector3 in the Babylon.js codebase. It also illustrates the loss of type safety from switching from a member function to a static method.

2. Converting the underlying storage to shared structs

https://github.com/kbabbitt/Babylon.js/tree/shared-structs-2-underlying-storage

In this branch I've modified Vector3 to use a shared struct rather than member variables for storage. I've also added methods to flatten and unflatten Vector3 and partial implementations of the same on Particle. In point 2 above I alluded to classes needing to know about each others' internals, but this turned out not as bad as I'd feared. It is a bit unfortunate to have to maintain these functions as the shape of the class changs, and there's still a loss of type safety stemming from not yet having language-level support for shared structs.

This prototype also uncovered another problem. Babylon has a DeepImmutable generic that makes all properties of an object readonly recursively. Implementation helper DeepImmutableObject applies this generic to all members of an object, discovering them by iterating over the object's keys. However, we can't currently understand the shape of a shared struct at compile time, so this application fails. The result is compilation errors when assigning between Vector3 and DeepImmutable which occurs in a handful of places in the code.

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

No branches or pull requests

1 participant