Skip to content

Conversation

@layershifter
Copy link
Member

@layershifter layershifter commented Apr 30, 2021


Intro

This PR is an initial piece of the new bundle size tooling: currently it implements only local builds and reporting and adds few fixtures for existing packages.

  • works per package (allows scoping)
  • uses fixtures to measure different scenarios
  • relies on build artifacts (i.e. requires build first) as we want to test the same output as customers will consume

Fixture example

We want to test different scenarios (current tooling tests a single export). In an example below we can measure impact of build time version of makeStyles().

import { __styles, mergeClasses } from "@fluentui/react-make-styles";

console.log(__styles, mergeClasses);

export default {
  name: "makeStyles + mergeClasses (build time)"
};

Usage

$ bundle-size measure --help
bundle-size.js measure

builds bundle size fixtures and generates JSON report

Options:
  --help       Show help                                               [boolean]
  --quiet, -v  Suppress verbose build output          [boolean] [default: false]

Sample output from CLI

yarn workspace @fluentui/react-make-styles bundle-size
yarn workspace v1.22.0
yarn run v1.22.0
$ bundle-size measure
[i] artifacts dir is cleared
[i] Measuring bundle size for 3 fixture(s)...
  - bundle-size/makeStaticStyles-runtime.fixture.js
  - bundle-size/makeStyles-runtime.fixture.js
  - bundle-size/makeStyles.fixture.js
[i] "makeStaticStyles-runtime.fixture.js": Webpack in 0.92s
[i] "makeStaticStyles-runtime.fixture.js": Terser in 0.36s
[i] "makeStyles-runtime.fixture.js": Webpack in 0.27s
[i] "makeStyles-runtime.fixture.js": Terser in 0.54s
[i] "makeStyles.fixture.js": Webpack in 0.19s
[i] "makeStyles.fixture.js": Terser in 0.11s
┌────────────────────────────────────────┬───────────────┬───────────┐
│ Fixture                                │ Minified size │ GZIP size │
├────────────────────────────────────────┼───────────────┼───────────┤
│ makeStaticStyles (runtime)             │ 8.51 kB       │ 3.66 kB   │
├────────────────────────────────────────┼───────────────┼───────────┤
│ makeStyles + mergeClasses (runtime)    │ 22 kB         │ 8.34 kB   │
├────────────────────────────────────────┼───────────────┼───────────┤
│ makeStyles + mergeClasses (build time) │ 2.82 kB       │ 1.34 kB   │
└────────────────────────────────────────┴───────────────┴───────────┘
Completed in 2.75s

@size-auditor
Copy link

size-auditor bot commented Apr 30, 2021

Asset size changes

⚠️ Insufficient baseline data to detect size changes

Unable to find bundle size details for Baseline commit: 8ddef3b

Possible causes

  • The baseline build 8ddef3b is broken
  • The Size Auditor run for the baseline build 8ddef3b was not triggered

Recommendations

  • Please merge your branch for this Pull request with the latest master build and commit your changes once again

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 30, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 25b135e:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration
codesandbox-react-northstar-template Configuration

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 30, 2021

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 979 989 5000
BaseButton mount 1011 1027 5000
Breadcrumb mount 2994 2921 1000
ButtonNext mount 572 569 5000
Checkbox mount 1655 1705 5000
CheckboxBase mount 1415 1447 5000
ChoiceGroup mount 5318 5269 5000
ComboBox mount 1115 1077 1000
CommandBar mount 10976 11093 1000
ContextualMenu mount 6537 6743 1000
DefaultButton mount 1248 1239 5000
DetailsRow mount 4156 4165 5000
DetailsRowFast mount 4042 4113 5000
DetailsRowNoStyles mount 3969 3846 5000
Dialog mount 2292 2350 1000
DocumentCardTitle mount 175 174 1000
Dropdown mount 3599 3543 5000
FocusTrapZone mount 1938 1947 5000
FocusZone mount 1961 1876 5000
IconButton mount 1985 1979 5000
Label mount 356 354 5000
Layer mount 1992 2022 5000
Link mount 531 537 5000
MakeStyles mount 1941 1936 50000
MenuButton mount 1661 1691 5000
MessageBar mount 2172 2213 5000
Nav mount 3499 3593 1000
OverflowSet mount 1115 1109 5000
Panel mount 2223 2232 1000
Persona mount 932 904 1000
Pivot mount 1549 1557 1000
PrimaryButton mount 1438 1460 5000
Rating mount 8623 8467 5000
SearchBox mount 1455 1502 5000
Shimmer mount 2942 2843 5000
Slider mount 2140 2158 5000
SpinButton mount 5483 5463 5000
Spinner mount 465 472 5000
SplitButton mount 3486 3532 5000
Stack mount 557 565 5000
StackWithIntrinsicChildren mount 1757 1773 5000
StackWithTextChildren mount 5249 5222 5000
SwatchColorPicker mount 11271 11395 5000
Tabs mount 1686 1567 1000
TagPicker mount 2671 2757 5000
TeachingBubble mount 12664 12773 5000
Text mount 476 454 5000
TextField mount 1515 1521 5000
ThemeProvider mount 1291 1279 5000
ThemeProvider virtual-rerender 643 631 5000
ThemeProviderNext mount 7066 7255 5000
Toggle mount 902 904 5000
buttonNative mount 124 116 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 193 167 1.16:1
BoxMinimalPerf.default 443 385 1.15:1
FlexMinimalPerf.default 343 313 1.1:1
VideoMinimalPerf.default 737 686 1.07:1
DialogMinimalPerf.default 856 809 1.06:1
LayoutMinimalPerf.default 432 407 1.06:1
RefMinimalPerf.default 260 246 1.06:1
AnimationMinimalPerf.default 480 456 1.05:1
ChatDuplicateMessagesPerf.default 337 322 1.05:1
TreeWith60ListItems.default 206 196 1.05:1
AvatarMinimalPerf.default 234 225 1.04:1
CardMinimalPerf.default 655 630 1.04:1
ListWith60ListItems.default 733 707 1.04:1
LoaderMinimalPerf.default 791 758 1.04:1
SkeletonMinimalPerf.default 412 395 1.04:1
SliderMinimalPerf.default 1788 1722 1.04:1
TableMinimalPerf.default 485 465 1.04:1
ButtonSlotsPerf.default 639 623 1.03:1
ImageMinimalPerf.default 431 420 1.03:1
ItemLayoutMinimalPerf.default 1410 1375 1.03:1
LabelMinimalPerf.default 447 435 1.03:1
ProviderMergeThemesPerf.default 1786 1737 1.03:1
ProviderMinimalPerf.default 1125 1097 1.03:1
RadioGroupMinimalPerf.default 524 509 1.03:1
ReactionMinimalPerf.default 452 438 1.03:1
SplitButtonMinimalPerf.default 4277 4153 1.03:1
DropdownMinimalPerf.default 3369 3295 1.02:1
HeaderSlotsPerf.default 880 860 1.02:1
ListCommonPerf.default 729 717 1.02:1
ListNestedPerf.default 667 652 1.02:1
AttachmentSlotsPerf.default 1292 1280 1.01:1
ChatMinimalPerf.default 690 682 1.01:1
FormMinimalPerf.default 480 476 1.01:1
InputMinimalPerf.default 1377 1364 1.01:1
ListMinimalPerf.default 570 563 1.01:1
MenuMinimalPerf.default 933 922 1.01:1
RosterPerf.default 1289 1270 1.01:1
PopupMinimalPerf.default 606 600 1.01:1
StatusMinimalPerf.default 780 770 1.01:1
IconMinimalPerf.default 683 675 1.01:1
TooltipMinimalPerf.default 1102 1095 1.01:1
ButtonOverridesMissPerf.default 1845 1852 1:1
CarouselMinimalPerf.default 517 516 1:1
ChatWithPopoverPerf.default 401 400 1:1
CheckboxMinimalPerf.default 3000 3010 1:1
DividerMinimalPerf.default 407 409 1:1
GridMinimalPerf.default 386 387 1:1
TableManyItemsPerf.default 2200 2200 1:1
TextMinimalPerf.default 392 392 1:1
TextAreaMinimalPerf.default 566 565 1:1
CustomToolbarPrototype.default 4073 4074 1:1
ToolbarMinimalPerf.default 1074 1073 1:1
AccordionMinimalPerf.default 183 185 0.99:1
ButtonMinimalPerf.default 199 201 0.99:1
EmbedMinimalPerf.default 4480 4527 0.99:1
HeaderMinimalPerf.default 412 417 0.99:1
MenuButtonMinimalPerf.default 1746 1759 0.99:1
AlertMinimalPerf.default 319 324 0.98:1
DropdownManyItemsPerf.default 755 767 0.98:1
PortalMinimalPerf.default 173 176 0.98:1
SegmentMinimalPerf.default 410 418 0.98:1
DatepickerMinimalPerf.default 6424 6600 0.97:1
TreeMinimalPerf.default 872 898 0.97:1

@layershifter layershifter marked this pull request as ready for review May 18, 2021 11:24
@layershifter layershifter requested a review from a team as a code owner May 18, 2021 11:24
@layershifter
Copy link
Member Author

Future progress is inside #18256:

  • compare-reports command with CLI & Markdown (for CI) reporters
  • upload-report command to upload results for master
  • integration with CI

A sample report output 👇

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-divider
Divider
17.892 kB
5.83 kB
18.266 kB
5.941 kB
374 B
111 B
react-image
Image
11.521 kB
4.344 kB
10.784 kB
4.238 kB
-737 B
-106 B
react-make-styles
makeStaticStyles (runtime)
8.27 kB
3.566 kB
8.514 kB
3.66 kB
244 B
94 B
react-make-styles
makeStyles + mergeClasses (runtime)
21.926 kB
8.326 kB
22.994 kB
8.841 kB
1.068 kB
515 B
react-make-styles
makeStyles + mergeClasses (build time)
2.67 kB
1.263 kB
2.823 kB
1.342 kB
153 B
79 B

🤖 This report was generated against abcd

Copy link
Contributor

@Hotell Hotell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! few questions/suggestions that are non blocking and can be addressed as a followup.

can we add tests please ?


- `[fixture].fixture.js` - a modified fixture without a default export, used by a bundler
- `[fixture].output.js` - a partially minified file, useful for debugging
- `[fixture].min.js` - a fully minified file, used for measurements
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: AFAIR we wanted also gzipped/brotli outputs. will this land in future PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently I implemented only GZIP via gzip-size:

const gzippedSize = await gzipSize.file(terserOutputPath);

We can add brotli in the same way.


Honestly, I don't know if these is sense to produce [fixture].min.js.gz in artifacts dir. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah in general having gzip or brotli isn't that much helpful. I mean we don't wanna test how compression algorithms works. For our packages, IMO, the only important info that we need is minified size, (gzip is solely for "marketing" purposes :) )

*
* @return {import("webpack").Configuration}
*/
function createWebpackConfig(fixturePath, outputPath) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: does this mean that the tooling is tightly coupled to webpack?

Copy link
Member Author

@layershifter layershifter May 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can switch later but as Webpack is a default tool for customers currently we should measure with it.


For example, we can use Rollup (it definitely does better job with tree shaking), but then output for Webpack could be heavier for 10kb for unknown reason and will not notice it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, It would be nice to have base benchmark based on rollup/esbuild to follow industry standard for packages and webpack as a 3rd party check for costumers (this can get out of hand anyway as we are not in charge or customers deps nor their tooling setup).

But we can make it pluggable later... thx

Copy link
Contributor

@Hotell Hotell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added few other suggestions regarding docs.

In general I would like to resolve the acceptance criteria for future build output (comment about relative paths within fixtures), but I don't wanna block.

thank you !


// ---

/** @type {import('yargs').CommandModule} */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

@@ -0,0 +1,7 @@
import { __styles, mergeClasses } from '@fluentui/react-make-styles';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, ty.

this keeps me wondering, how would this work with some future adjustment that I (you as well) mentioned few times regarding build output (ref).

context

  • having single /dist in root
|- dist/
|- |- <package-a>/
|- |- |- index.js
|- |- |- index.umd.js
|- |- |- index.d.ts
|- |- |- package.json

|- |- <package-b>/

meaning we'd need to do following (for local fixtures):

import { __styles, mergeClasses } from '../../../dist/react-make-styles';

will this work ?

@layershifter layershifter enabled auto-merge (squash) May 25, 2021 09:59
@layershifter layershifter merged commit 8b08552 into master May 25, 2021
@layershifter layershifter deleted the chore/bundle-size branch May 25, 2021 11:17
"extends": ["plugin:@fluentui/eslint-plugin/node"],
"root": true,
"rules": {
"import/no-extraneous-dependencies": "off"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Late comment but you can probably fix this by adding the following setting.

  "settings": {
    "import/resolver": {
      "typescript": {
        "project": "../../tsconfig.json"
      }
    }
  },

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, it does not work 😭 It's applied to import/no-extraneous-dependencies rule that has own dependencies and could be tweaked via packageDir:

{
  "import/no-extraneous-dependencies": [
    "error",
    { "packageDir": "./some-dir/" }
  ]
}

I will apply this change in #18322. Thanks!

"bundle-size": "./bin/bundle-size.js"
},
"scripts": {
"lint": "just-scripts lint",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also a late comment (sorry) but since you're not using just for anything else, you might as well remove the just config and call eslint directly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... Nice, also applied in #18322 👍

@layershifter layershifter mentioned this pull request May 26, 2021
6 tasks
@Hotell Hotell added this to the May Project Cycle Q2 2021 milestone Jul 13, 2021
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.

8 participants