mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
101.79k stars 35.31k forks source link

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

Closed bhouston closed 3 years ago

bhouston commented 3 years ago

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. https://github.com/mrdoob/three.js/issues/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:

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.

arodic commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago
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 commented 3 years ago

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 https://github.com/mrdoob/three.js/issues/21667#issuecomment-821789762, 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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

I got pulled away so I will not have time to do this myself. I apologize. Please someone else take this on if there is community interest.

mrdoob commented 3 years ago

Closing this then.