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

Make math directory tree-shakable (inspired by threeify's approach) #21667

Closed
bhouston opened this issue Apr 16, 2021 · 14 comments
Closed

Make math directory tree-shakable (inspired by threeify's approach) #21667

bhouston opened this issue Apr 16, 2021 · 14 comments

Comments

@bhouston
Copy link
Contributor

bhouston commented Apr 16, 2021

Is your feature request related to a problem? Please describe.

Currently the math library is taking up a significant amount of space in the Three.js final bundle. 125KB of 666KB - so roughly 20% of the space.

This is clear from this image shared on Twitter: https://twitter.com/threejs_org/status/1382639620119216131

EzAexmnXEAEFA7l

This is unnecessary. The size of Three.js is larger than it needs to be because all math functions are part of classes and they reference each other as functions of classes. While it is good to have classes for Vector3, Matrix4, etc, having every single function that operators on these classes as a member of a class makes it very hard to tree shake the math classes -- in fact it is straight out impossible by design.

This is the second of two feature requests on how to reduce further Three.j's build sizes based on my learnings from threeify. #21665

Describe the solution you'd like

Instead we should try to demote any significant complex math function that operates on these math classes to a separate standalone function. This function has to be explicitly imported and used. Then when these functions are not required, they will be tree shaken out of the build. I did this approach with Threeify's math library and it was very beneficial in reducing build sizes.

Specifically I split the classes like Vector3 into Vector3.js and Vector3.Functions.js, where the Vector3.Functions file contained everything that was tree-shakable:
https://github.com/threeify/threeify/tree/master/src/lib/math

One can be very aggressive in doing this or less aggressive. I think some simple functions to do this with would be the large ones that are not always used or that unnecessarily tie one math class to another. Basically changes similar to this across the library would do wonders:

  • Vector3.applyAxisAngle - should be just applyAxisAngleVector3().
  • Vector3.applyMatrix3 - should just be applyMatrix3Vector3().
  • Vector3.applyNormalMatrix - should just be applyNormalMatrixVector3().
  • Vector3.applyQuaternion - should just be applyQuaternionVector3().

The above functions when part of Vector3, cause Vector3 to automatically import Quaternion and Matrix3 whether Vector3 is used whether these other classes are needed or not. But converting them to standalone functions now only brings in those other classes when they are truly needed.

By taking this approach to the whole math library, this can reduce the minimum build size of Three.js to what is just actually required.

For threeify, this means that final build sizes can be as small as 18KB once minimized and compressed. Wouldn't it be nice to see ThreeJS approach this size of output? https://threeify.org/examples/brdf_clear_coat_specular

Describe alternatives you've considered

The main downside of the above approach is that it will cause backwards compatibility issues. This is a significant concern.

A major upside to this approach is that we could include a lot more useful types of math functions in the core of tHree.js because they will only be included in the bundles if they are used. Right now we are very careful about what goes into the math library because it isn't tree-shakable, but if we did the approach described here, it would change our calculations.

@bhouston bhouston changed the title How to reduce math library size: move specialized functions of out classes How to enable math library tree-shaking by moving specialized functions of out classes Apr 16, 2021
@bhouston bhouston changed the title How to enable math library tree-shaking by moving specialized functions of out classes Make math directory tree-shakable (inspired by threeify's approach) Apr 16, 2021
@arodic
Copy link
Sponsor Contributor

arodic commented Apr 16, 2021

While I like the idea of tree shakable math library it is important to note that this will greatly increase the verbosity of import statements. For example:

import {
  Vector3,
}

Will become:

import {
  Vector3,
}
import {
  multiplyScalarVector3,
  multiplyVectorsVector3,
  applyEulerVector3,
  applyAxisAngleVector3,
  applyMatrix3Vector3,
  applyNormalMatrixVector3,
  applyMatrix4Vector3,
  applyQuaternionVector3,
  ...
} from 'Vector3Functions';

That makes me wonder if ~50-100KB convenience fee (status quo) is actually worth it.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 16, 2021

Good point. Tree-shakebility is important but we should not declare it to the most important design goal. I personally favor readability and ease of use. And verbose import statements seem counterproductive in that regard.

@bhouston
Copy link
Contributor Author

bhouston commented Apr 16, 2021

So I implemented this pattern in threeify. Here are a number of common use cases where you can see this pattern used:

What I basically found is that these advanced functions are rarely used. You may think you use them a lot, but basically they are almost completely unused in the majority of the code. This is why the tree shaking works so well in threeify.

You probably think you use them a lot, and maybe in a few cases you do, but you are paying 100KB for that one or two cases. Could you point out where these are most used? What is this case that the hassle of all these imports is worth paying 100KB of data all the time?

PS. multiplyScalarVector3(), multipleVectorVector3 would not pull in other classes, thus do not make them independent functions as it doesn't really achieve much. I didn't in my implementation: https://github.com/threeify/threeify/blob/master/src/lib/math/Vector3.ts

@bhouston
Copy link
Contributor Author

@arodic can you share with me the places where you would have more than a few imports for the math functions? I think they are rare in the majority of the code. I worry that this is a bit of a straw man so I do want to call you on it.

@arodic
Copy link
Sponsor Contributor

arodic commented Apr 17, 2021

@bhouston I didn't really search for examples as I misunderstood that all functions would be separated per your suggestion. So you are saying only the "advanced" or less commonly used functions would be separated. In that case I guess a few additional imports here and there are not that bad.

How would we decide which function lives in the core class and which one is standalone? Is there an intuitive way to draw that line?

@gsimone
Copy link
Contributor

gsimone commented Apr 17, 2021

A first step could be to move away all the functions that reference another module, so for example Vector3.applyNormalMatrix and Vector3.applyQuaternion, so that code that doesn't use them wouldn't import the Matrix or Quaternion modules - I assume this was @bhouston's idea. This would probably have a positive snowball effect without changing too much of the API or common code patterns. There might be some common cases where this assumptions breaks, can't think of examples right now

TLDR: if ModuleA uses ModuleB in a method, move that function to a third separate function so that ModuleA doesn't have to depend on ModuleB.

@drcmda
Copy link
Contributor

drcmda commented Apr 17, 2021

import {
  Vector3,
}

Will become:

import {
  multiplyScalarVector3,
  multiplyVectorsVector3,
  ...
import * as Vector3Math from 'Vector3Functions'

Vector3Math.multiplyScalarVector3(...
Vector3Math.multiplyScalarVector3(...

imo it would be the exact same as import * as THREE from 'three', tree shaking should still work in that case.

That makes me wonder if ~50-100KB convenience fee (status quo) is actually worth it.

that is seconds added TTL in countries where bandwidth is low or expensive. imo no dev convenience is worth that.

@donmccurdy
Copy link
Collaborator

For threeify, this means that final build sizes can be as small as 18KB once minimized and compressed. Wouldn't it be nice to see ThreeJS approach this size of output? https://threeify.org/examples/brdf_clear_coat_specular

Could you say more about what is actually included in a bundle this small? Eliminating 20% of total bundle size obviously sounds great, but I'll admit I'm a bit skeptical that typical usage (i.e. needing a WebGLRenderer, a scene graph, at least one material, and perhaps a loader and controls) requires so little of the current math library.

The obvious point of comparison here is https://glmatrix.net/, which took a functional approach from the beginning. To be honest, I find it to be much less user friendly than the three.js math API, and not because it isn't well designed — three.js features like method chaining make code more readable.

I think we need to understand realistic bundle size gains here, rather than an upper bound.


To @gsimone's comment #21667 (comment), as long as Vector3, Matrix4, etc. classes exist, we should probably expect that bundles will include them. i.e. the Matrix4 class will never be tree-shaken (unless tree-shaking someday prunes individual class methods?), because WebGLRenderer is always going to need to do some matrix operations. So, if we are going this direction, the priority should probably be to create separate exports from methods that are currently less commonly used and/or larger.

@bhouston
Copy link
Contributor Author

bhouston commented Apr 19, 2021

Could you say more about what is actually included in a bundle this small?

It is an "immediate mode" renderer (as opposed to "retained mode" which is how Three.js is fundamentally designed, details here: https://docs.microsoft.com/en-us/windows/win32/learnwin32/retained-mode-versus-immediate-mode) which means it doesn't use a SceneGraph nor a complexMaterial system. Rather it uses Programs and Meshes individually. That said, adding a SceneGraph or a complex Material system isn't that much work, usually one builds retained mode renderers onto of an underlying immediate mode system.

https://github.com/threeify/threeify/blob/master/src//examples/brdfs/clearcoat/index.ts

(The idea was to put in specific "Engines" which may be various methods of "Retained mode" implementation in the /engine directory in threeify. Thus splitting the core immediate mode from retained modes, of which I believe there can be many.)

I think we need to understand realistic bundle size gains here, rather than an upper bound.

Both shaders and the math library are tree shake-able in threeify. But given that both the shaders and the math library make up 40% of the current bulk of Three.js it is likely we can significantly reduce the bundle size from where it is.

In the past we have had significant discussions about whether to include one Matrix4 function or not because we know that our bundling strategy causes it to be included everywhere. But if we split the complex ones into functions, this would be a moot point. We can add the decompositions like Eigen, SVD, etc that I've wanted forever and other more advanced topics as helper functions and they will feel like a natural part of the library.

@aardgoose
Copy link
Contributor

Could you for backward compatibility, add a legacy mode that optionally adds the advanced functions via a shim to pass 'this' on to the original objects Vector3.prototype etc as required?

@bhouston
Copy link
Contributor Author

Could you for backward compatibility, add a legacy mode that optionally adds the advanced functions via a shim to pass 'this' on to the original objects Vector3.prototype etc as required?

I think that is required. Yes. So in the next few days I'll make a small PR that shows the change on a few classes/functions and we can decide whether it is worthwhile pushing it to completion.

@munrocket
Copy link
Contributor

Agree with this, also WebXR is useless on 95% of sites but this JS code exists there. Since threejs don't make XR fallback by default it can be optimized for three-shaking.

@bhouston
Copy link
Contributor Author

bhouston commented May 30, 2021 via email

@mrdoob
Copy link
Owner

mrdoob commented Jun 2, 2021

Closing this then.

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

No branches or pull requests

9 participants