danvk / source-map-explorer

Analyze and debug space usage through source maps
Apache License 2.0
3.82k stars 100 forks source link

Show gzipped file sizes. #123

Closed tylergraf closed 4 years ago

tylergraf commented 5 years ago

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

coveralls commented 5 years ago

Coverage Status

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

nikolay-borzov commented 5 years ago

I was thinking about providing both stat and gzipped sizes.

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

The more flags the more complex app becomes

tylergraf commented 5 years ago

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

nikolay-borzov commented 5 years ago

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

danvk commented 5 years ago

I'd vote for a command line flag or option in the UI to toggle this. Showing both numbers in each box might be a bit much.

It's not always clear how to account for a module's contribution to the gzipped size. If I depend on A@1 and on B, which depends on A@2, then both versions of A will take similar amounts of space in the bundle. But presumably they're similar to one another and gzip(A@1 + A@2) will be similar in size to gzip(A@2). So how do you account for this? Is B's dependence on A free?

On Fri, Jul 5, 2019 at 12:40 PM Nikolay Borzov notifications@github.com wrote:

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

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/danvk/source-map-explorer/pull/123?email_source=notifications&email_token=AAAX77LQ2DE6FPXQW6G5JE3P552QVA5CNFSM4H5MPDTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZJ5XUA#issuecomment-508812240, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAX77NLIZ5KMLOO5QNBJTLP552QVANCNFSM4H5MPDTA .

tylergraf commented 5 years ago

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

Hey guys, any thoughts on this?

nikolay-borzov commented 5 years ago

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

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

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

danvk commented 5 years ago

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

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

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".