ethereumjs / ethereumjs-monorepo

Monorepo for the Ethereum VM TypeScript Implementation
2.57k stars 742 forks source link

Monorepo -> Tree Shaking Refactor: Analyze esbuild Output Code #3459

Open holgerd77 opened 2 months ago

holgerd77 commented 2 months ago

Part of #3446

Earliest start together with official breaking release work!

When analyzing the code produced with esbuild respectively - to name it simpler - just scrolling through the produced code, e.g. when building the blockchain example from [here](npm run build && esbuild --bundle 08Blockchain.mjs --platform=browser --outfile=out1.mjs --format=esm) with npm run build && esbuild --bundle 08Blockchain.mjs --platform=browser --outfile=out1.mjs --format=esm it is totally mindblowing to see what's all in there together with the intuition/suspicion one is imminently getting that the very very most parts of this code will never be applied! 😋

We should definitely leverage this as well as an approach, this gives a very direct and clear feeling for the good, the bad and the ugly (for Blockchain I have e.g. discovered that the full Node.js Buffer code is still included there, still scratching my head about this 🤔).

To get some feeling, output code from these generated files look like this:

grafik

Or another example:

grafik

My first-round impression is there is a lot of room for improvement, also pointing to things going beyond the tree shaking topic.

acolytec3 commented 1 month ago

@roninjin10 question for you on tree-shaking. I've been doing some experiments with export maps that specifically define all of the entry points for a given module (specifically with statemanager as a starting point). When I define an export map in package.json as something like:

"exports: {
  './statemanager': {
"import": "dist/esm/stateManager.js"
},
'./rpcStateManager': {
"import": "dist/esm/rpcStateManager.js"
}
...

and take out the root export, vite seems to be very successful at excluding both internal code from statemanager as well as it's dependency tree when I bundle statemanager in a react app, like this: image

vs like this when I have a single entry point '.': { "import": "dist/esm/index.js"} image

In the second instance, only the unused code within the statemanager package is excluded while a whole bundle of transitive deps from the statemanager module that aren't used by the specific import in my react app (i.e. simpleStateManager.js) get bundled.

Do you like export maps as the main way of specifying entry points in modules to assist with tree-shaking? I haven't tested with other bundlers/frameworks but this seemed like it worked really really well and maybe should be our goal with the next round of breaking releases (i.e. to remove the default entry point on all of our downstream libraries and specify named entry points to reduce the overall bundle size for consuming apps).

roninjin10 commented 1 month ago

TLDR

For the most part, No, export maps do not affect tree shaking.

Example

If I am an end user using webpack vite etc. and my bundler sees this:

import { rpcStateManager } from '@ethereumjs/state-manager'

It will step into that file

// index.js
export { rpcStateManager } from './rpcStateManager.js'
export {stateManager} from './statemanager.js'

and then it will immediately tree shake it

// index.js
export { rpcStateManager } from './rpcStateManager.js'
// remove unused export
// export {stateManager} from './stateManager.js'

After it tree shakes it the bundle hit will be the same size as it is when you use exports to seperate.

Exception

There are edge cases. For a UI like metamask that uses lavamoat, lavamoat doesn't support tree shaking. If a user isn't tree shaking themselves these seperate entrypoints are useful in this edge case because you essentially tree shaked for them.

Testing this

If you want to test how it looks like tree shaking what you can do is create a new file treeShakingTest.ts

import {rpcStateManager} from './index.ts'

Change that bundle analyzer to use this as the entrypoint and you will see how it will behave in this situation. Pointing it at index.js is the equivelent to doing this

import * as stateManagerModule from '@ethereumjs/state-manager'

Also I really like this online tool for doing this: .

image
holgerd77 commented 1 month ago

Ok, but guess we really need to have a close look why this didn't work with Andrews experiments, if I understood correctly he more or less did the same thing (do a file with a specific import and then check the resulting bundle size).

At the moment I perceive these results as contradictory and not giving a clear direction yet. 🤔

roninjin10 commented 1 month ago

I can look. Could be a bunch of things preventing tree shaking.

Are side effects marked as false? https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free

roninjin10 commented 1 month ago

Gave it a look and the reason tree shaking isn’t working in state manager package is this part of package.json

"type": "commonjs",

Change this to module and it should work. Cjs doesn’t tree shake

holgerd77 commented 1 month ago

@roninjin10 ah, yeah, that's very much on the plate/horizon anyhow to do the full switch and also go ESM for our internal setup, that's somewhat of a left-over! Guess we should put this very much of the front of the breaking release work, this was in the way of various stuff already and making things more complicated than it should be!

acolytec3 commented 1 month ago

Gave it a look and the reason tree shaking isn’t working in state manager package is this part of package.json

"type": "commonjs",

Change this to module and it should work. Cjs doesn’t tree shake

I'm not sure this is applicable. Our build process produces ESM and CJS outputs. I tried the same process I outlined above of switching statemanager to use an export map with named exports with webpack and it produced similar results to vite where it excluded whole dependency subtrees for the named exports that weren't imported by the consuming application so this feels like it will greatly enhance tree shaking for users of our libraries.

roninjin10 commented 1 month ago

Trust me on this. I say this with 100% confidence export maps do not affect tree shaking and if it looks like they do that means tree shaking failed to happen.

This can be tested using a library that has export maps like viem

roninjin10 commented 1 month ago

Tested this in a few instances to prove it. Setup here is a vite app that gets created with create wagmi

For zustand importing createStore from entyrpoint or vanilla are same

image

Same is true for a couple of other libraries I tested except for viem which showed a small but minimal difference of 0.2kb that didn't get tree shaken from entrypoint import

Our build process produces ESM and CJS outputs.

This could be true. Adding a console.log to the ESM imports only and then running the code is easiest way to test that it's properly using ESM. If it's is properly using ESM double check that "sideEffect": false is set in package.json

roninjin10 commented 1 month ago

Testing tree shaking with Tevm I noticed ethereumjs tree shakes way differently depending on how aggressive or loose the tree shaking settings are set

Below we see that if I set tree shaking to smallest it tree shakes all JS while if I set it to recomended it includes almost all ethereumjs code.

This implies that some rule that "smallest" doesn't follow is the root cause. Docs about what rules smallest ignores are here. SideEffect: false in package.json would be my first guess

telegram-cloud-photo-size-1-4933710584994704417-y

roninjin10 commented 1 month ago

The above code is using @ethereumjs/util as a sub dep of @tevm/utils I forgot to mention