-
Notifications
You must be signed in to change notification settings - Fork 1
perf: reduce Open Graph module bundle size #150
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
Merged
davidlj95
merged 2 commits into
main
from
stacked/perf-reduce-Open-Graph-module-bundle-size
Jan 13, 2024
Merged
perf: reduce Open Graph module bundle size #150
davidlj95
merged 2 commits into
main
from
stacked/perf-reduce-Open-Graph-module-bundle-size
Jan 13, 2024
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
📦 Bundle size (Angular v15)Git ref: 92ad8f0
|
📦 Bundle size (Angular v16)Git ref: 92ad8f0
|
📦 Bundle size (Angular v17)Git ref: 92ad8f0
|
6919cd6
to
3c0c5fd
Compare
5a153f7
to
92ad8f0
Compare
Closed
Closed
🎉 This PR is included in version 1.0.0-alpha.17 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This was referenced Jan 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The only thing preventing myself from using my own library was performance. This modularity that has been built takes some bytes. And those bytes make the main bundle bigger. And a bigger bundle means it will take longer to fetch from the network & parse. Hence increasing the First Contentful Paint (FCP) as the main JS bundle blocks loading of the page. Or Time to Interactive (TTI) if FCP is solved by using server side rendering (SSR)
Tracking bundle size
To reduce the size of the bundle, first added tools to the CI to keep track of bundle size ( see #123 ). Then, when we're able to monitor that info, we can start optimising.
Optimisations
Then, inspected where are the bytes going. To do so, inspected the main JS bundle of the Angular v17 app production build. It includes the main features of the library due to the E2E tests. This means building the main JS bundle with
esbuild
. After inspecting minified bundles, discovered that:@Injectable
made some magic Angular bytes appear there to allow injectionSee #112 Totally understandable. First try was to join all metadata per group (Open Graph, Twitter card, ...) in same class, so that code only exists once per module. However, see next point to why that's not ideal. Also doing it that way, we don't allow authors to manually remove certain metadata setters that won't ever be applied when injecting those manually.
To solve it, using factory providers instead. Indeed, a function that makes a factory provider, to avoid repeating the provider declaration around.
Classes cannot be easily optimised
First, they add some constructs
class X extends Y
that take some bytes.Secondly, unused methods aren't removed given bundlers like
esbuild
(the one in v17), don't provide method devirtualization advanced optimisations. Indeed, that's why Angular uses Google's Closure Compiler. I assume others likewebpack
(used in Angular til v16) don't do. I recall checking it but not sure now tbh.They also keep the names of class properties. Even if they're marked as
private
in Typescript (as that info is lost when compiling from TS to JS, so at bundling time it's not there anymore). And those can get quite long. You can mangle properties using rules when building but Angular doesn't provide easy access to the builder inner options (likeesbuild
opts) by design. So even if managing to do so, would be against Angular design and don't want to maintain that.That's why functions are used instead of classes. Sorry OOP, I like you, but I prefer performance 😬. Also, SOLID practices can still be enforced using functions instead of classes anyway.
Functions have no
class X extends Y
syntax. Indeed, we can use arrow syntax which is super concise (instead offunction
). They also have no props. And argument names can be shortened when minifying. So if providing something as argument instead of being a class property, it can be renamed to something short. For instancesuperDuperLongService
to a random short name likeaX
.Object property names are not shortened
Minification process shortens names of identifiers (vars, functions, etc...) that are not part of global scope. However doesn't shorten object property names. Given they may be accessed with braces syntax like
obj['foo']
. So not safe to shorten them & ensure code runs properly given those dynamic use cases allowed. Classes will be instantiated into objects. So those method names / attributes won't be shortened. That's indeed whyprivate
typescript methods aren't shortened either. Cause that info is lost when compiling into JS. And it's JS the one being minified, so at that point minifier isn't sure if that's public or private and avoids shortening the name. Could try with JS private attributes tho 🤔Anyway, property names are being kept as short as possible. Or removed to use
const
s outside of global scope that can be shortened.Two equal literal strings aren't declared just once
They're declared many times. TBH not sure why. Maybe cause the minification doesn't go so far as they state in advanced optimizations section. So when we know two strings will be the same, we can create a
const
and refer to it to avoid that string literal appearing multiple times.Experiment
Trying with Open Graph given it's mostly just setting
og:
meta properties. So shouldn't take the size it takes right now (2.7kBs). Code generated shouldn't be much more than the name of the JSON prop to look at + OG property to set. Which may be the same.Result
See for yourself: #150 (comment) 🤭 🎉
65% (62% in webpack builds) of Open Graph module bytes are gone 👋 At cost of adding 124 bytes (3%) to core module. So 59% effective gain. And that's not even considering compression like
gzip
orbrotli