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

Implement vectors without proxy #779

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

deluksic
Copy link
Contributor

@deluksic deluksic commented Jan 14, 2025

Instead of using Proxy to implement swizzling, we can use new Function to compile small getters with no runtime if / switch. This should allow js engines to optimize these calls better, but more testing is needed. The most important thing is that vec[0] and vec.x are simple index access and a getter respectively.

Example swizzle getter:

get xxyw() {
  return new this._Vec4(this[0], this[0], this[1], this[3]);
}

One important change is the ability to unpack vectors like:

const [x, y] = v2

I achieve this by extending Array. Typescript doesn't understand yet that this is possible, but I think we should be able to tell it.

My first tests show almost 2x speedup in buffer.write speed, 25ms vs 40ms for 50k Circle(position, radius, styleIndex) structs.

@deluksic deluksic force-pushed the deluksic/vector-no-proxy branch 2 times, most recently from af23f7e to ca3c797 Compare January 14, 2025 01:20
Copy link

codesandbox-ci bot commented Jan 14, 2025

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

}

if (values.length === options.length) {
if (values.length <= 1 || values.length === options.length) {
return options.make(...values);
Copy link
Contributor Author

@deluksic deluksic Jan 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably, we could pass the values directly to the constructor of the vector and not check lengths. In case someone generates thousands of vectors, these checks can add up. We could simply rely on typescript to handle this for us and not pay the runtime cost (even if small).

I think having the option to initialize using vec4(vec3(), w) would be super cool, but I did not find a way to do this without paying the cost of checking args. If someone wants to do this in js, the simplest way would be to vec4(...vec3(), w). This would work just fine, we just need to adjust the transpiler to ignore the dots.

@iwoplaza
Copy link
Collaborator

Awesome initiative! 👏
We have to see if creating functions on-demand works well in Hermes (React Native). If not, we can always do the generation as a build step, and ship the generated code in the package.
I'll take a closer look in the coming days 👀

@deluksic
Copy link
Contributor Author

If it can be autogenerated on build for everyone, that would be even better! Although downsides are code size, and not being able to use the library from node without compile step. (Node has experimental type stripping support)

@iwoplaza
Copy link
Collaborator

To be able to validate if the generation works properly, I'd like to codegen to the file system, and commit. So even if a Node project would like to consume the library from source, it should work okay!

@deluksic
Copy link
Contributor Author

Oooh I see

@deluksic deluksic force-pushed the deluksic/vector-no-proxy branch from ca3c797 to 892aa9c Compare January 15, 2025 22:55
@@ -11,7 +11,7 @@ export default defineConfig({
outDir: 'dist',
format: ['cjs', 'esm'],
tsconfig: './tsconfig.json',
target: 'es2017',
target: 'es2022',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching to es2022 seems to improve performance of vector creation. I'm not sure why yet, but given WebGPU did not exist in es2017, we might as well switch it.

@iwoplaza
Copy link
Collaborator

Okay, I verified that the `new Function(...)` works fine on React Native 🎉. However, doing this at runtime still makes it impossible for things like Static Hermes or Porffor (JS compilers) to optimize them well AOT.

I would still attempt to offload this work AOT as a prepublish step, and ship the generated code. For now though, we can continue experimenting and pushing the performance with a JIT approach here, as it's easier to prototype with.

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

Successfully merging this pull request may close these issues.

2 participants