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

Show gzipped file sizes. #123

Merged
merged 5 commits into from Jan 25, 2020
Merged

Show gzipped file sizes. #123

merged 5 commits into from Jan 25, 2020

Conversation

tylergraf
Copy link
Contributor

Please excuse my naiveté.

I'd assume you'd want this as a flag passed in, I wasn't sure how to pass that param along.

Changes

  • While calculating the spans, concat the string of each source and then gzip that to get the size
  • I created the snapshots for the tests. I'm not sure if they're accurate. Please help. :)

@coveralls
Copy link

coveralls commented Jul 3, 2019

Coverage Status

Coverage increased (+0.09%) to 96.637% when pulling f92115d on tylergraf:gzip into fe52007 on danvk:master.

@nikolay-borzov
Copy link
Collaborator

I was thinking about providing both stat and gzipped sizes.

"totalBytes": [
	stat: 357,
	gzip: 90
]

The more flags the more complex app becomes

@tylergraf
Copy link
Contributor Author

Oh, ok. And just display both sets of data in the ui? I like that.

@nikolay-borzov
Copy link
Collaborator

Or we can add a dropdown (SIze: Stat, Gzip) that toggles displaying bundles with stat/gzip sizes as values

@danvk
Copy link
Owner

danvk commented Jul 5, 2019 via email

@tylergraf
Copy link
Contributor Author

@danvk I don't know the answer. I assume they'll be similar in size?

Also, I've updated the PR to show a select box at the top to select either gzip or stat.

@tylergraf
Copy link
Contributor Author

Hey guys, any thoughts on this?

@nikolay-borzov
Copy link
Collaborator

Sorry, I've been away from open source (and programming in general) for a while.
It needs more work. I don't like that adding gzip sizes had such impact on code base - there are many places that have changes. Perhaps we need to refactor some functions to reduce such impact (e.g. collect spans as characters and then calculate both stat size and gzip size). Also, the app should not by default output gzip (e.g. when outputting TSV) - we will have to add a flag that specifies preferred size type.
Give me this weekend and I'll try to make some changes. Otherwise, I will help you to make these changes.

@danvk
Copy link
Owner

danvk commented Jul 24, 2019

Thinking more on this, a simpler way to frame my concern is that a treemap visualization implies that sizes are additive. When you visualize raw files and directories, this is true. When you gzip everything, it's not.

@nikolay-borzov
Copy link
Collaborator

Do you mean that a sum of individual files gzip sizes won't be equal to bundle gzip size?

@danvk
Copy link
Owner

danvk commented Jul 24, 2019

Exactly. Or put another way, if you remove a 1k file from your JS bundle, it will get 1k smaller. But if you remove a file that's 1k after gzipping from your bundle, the gzipped bundle might not get 1k smaller.

It would help to understand the use case for showing gzipped sizes.

@sean9keenan
Copy link

sean9keenan commented Aug 16, 2019

I can chime in on why I'm interested in this. I care about the final size that goes over the network (as that's what most impacts users - which is why I love this tool so much!).

Since I normally gzip when deploying, I want to know which files/libraries I should target for removal. While you're right that removing a file that's 1k gzipped doesn't always mean my bundle gets 1k smaller - I'm hoping that gzipped sizes will provide a better proxy for final gzipped size than non-gzipped file sizes (In particular for files that are highly compressible).

Given the confusion around this I totally understand not wanting this to be the default, with perhaps a warning to the effect of "due to the nature of compression - removing a 1k gzipped file in a bundle may reduce the bundle size by less than 1k".

Ideally I'd like to know how much space I can save by optimizing away which files/libraries - but that seems like more work than is worth (especially if you want to be able to select any set of libraries for removal - and not just a single one).

Thanks for your work on this!

@tylergraf
Copy link
Contributor Author

I agree with @sean9keenan. I feel like it's better to be gzipped and close than to be technically right.

Almost everybody on my team has run the analyzer and wondered if the numbers were gzipped. The uncompressed number is valuable for knowing your memory footprint, but the most helpful info is to see gzipped information, even if it's not exact. Whenever I look at these tools, I always take them with a grain of salt anyway because they don't show what will be loaded on my page, they just show bundle sizes. My page will probably have several of the chunks.

So in my opinion, I'd like to get this feature in sooner rather than later, and just message the

"due to the nature of compression - removing a 1k gzipped file in a bundle may reduce the bundle size by less than 1k".

Add `gzip` option/parameter
Use `gzip-size` package to calculate gzip size
Set `onlyMapped` to `true` when `gzip` is set. Calculating gzip size make impossible to calculate unmapped bytes because the sum of gzip sizes is not equal to total bytes due to the nature of compression
Exclude `unmappedBytes` from the result if `onlyMapped` is set
Add a warning to visualization when `gzip` flag is set
@nikolay-borzov nikolay-borzov self-assigned this Jan 25, 2020
@nikolay-borzov nikolay-borzov added this to the 2.3.0 milestone Jan 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants