core(duplicated-javascript): display as transfer size #10701
Conversation
|
side note: @patrickhulce think this should be named |
|
🤷 eh I agree |
| @@ -85,156 +85,86 @@ describe('DuplicatedJavascript computed artifact', () => { | |||
| Object { | |||
| "source": "Control/assets/js/vendor/ng/select/select.js", | |||
| "sourceBytes": Array [ | |||
| 48513, | |||
| 48513, | |||
| 16009, | |||
There was a problem hiding this comment.
hm this feels weird to say that the sourceBytes are transferSize, no?
I agree with wasted computations using transferSize, that seems great 👍 (based on what was happening with wastedBytesByUrl I assumed they already were, and looks like that was half true and we were only using that for the opportunity savings and not the displayed wasted bytes?)
There was a problem hiding this comment.
hm this feels weird to say that the sourceBytes are transferSize, no?
sourceTransferBytes? I'm wondering if in the long run we'd want to modify the bytes type: {transfer: ..., resource: ...}.
based on what was happening with wastedBytesByUrl I assumed they already were, and looks like that was half true and we were only using that for the opportunity savings and not the displayed wasted bytes
yea, that map is only for the base class to calculate savings in latern. only showed up as the seconds savings estimate in the report.
There was a problem hiding this comment.
sourceTransferBytes sgtm 👍
There was a problem hiding this comment.
actually, given that wastedBytes and totalBytes on all opportunities are in terms of transfer size, I'm hesitant to break consistency in naming here. We don't specify transfer or resource bytes in any of our byte keys.
why does it feel weird?
There was a problem hiding this comment.
it feels weird here because we're specifically talking about the source code, unlike in all the other opportunities that are always talking about what was transferred. the introduction of source map data changes the game IMO because there's a completely new size we could be talking about, the original source code size. Calling something sourceSize makes me think it's the size of that original source code rather than the bytes that were transferred corresponding to a subset of that source code.
There was a problem hiding this comment.
sorry for the delay here @connorjclark I think this wound up mislabeled. were you waiting on me for anything else?
| @@ -85,156 +85,86 @@ describe('DuplicatedJavascript computed artifact', () => { | |||
| Object { | |||
| "source": "Control/assets/js/vendor/ng/select/select.js", | |||
| "sourceBytes": Array [ | |||
| 48513, | |||
| 48513, | |||
| 16009, | |||
There was a problem hiding this comment.
sourceTransferBytes sgtm 👍
| "wastedBytes": 4410, | ||
| }, | ||
| Object { | ||
| "source": "js/src/utils/service/amplitude-service.ts", |
There was a problem hiding this comment.
feels weird that so many of these disappeared, are they really being compressed that well they fall below 200 bytes?
There was a problem hiding this comment.
The estimated transfer size for this source is 1022, which is still above the 200 sub-item threshold.
It's elided because the per-item threshold is 1024 :)
There was a problem hiding this comment.
Lowered the threshold for this test.
There was a problem hiding this comment.
oooooh the changed threshold wasn't for this test, gotcha gotcha, sorry for the noise :)
| }, | ||
| "totalBytes": 0, | ||
| "url": "", | ||
| "wastedBytes": 3098, |
There was a problem hiding this comment.
like this one supposedly went from 3000+ bytes wasted to <200 bytes wasted?
in #10700 Patrick suggested I refactor the
sizeshere too. I realized that the table was mistakenly showing resource sizes.The opportunity savings estimate is from the
wastedBytesByUrlmap, which was correctly using transfer size, so estimated savings won't change.