Skip to content

feat(benchmarks): setup for benchmarking components #19320

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
merged 1 commit into from
Jun 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,12 @@ rbe_autoconfig(
# a specific Linux kernel that comes with "libx11" in order to run headless browser tests.
repository = "google/rbe-ubuntu16-04-webtest",
)

# Load pinned rules_webtesting browser versions for tests.
#
# TODO(wagnermaciel): deduplicate browsers - this will load another version of chromium in the
# repository. We probably want to use the chromium version loaded here (from dev-infra) as that
# one has RBE improvements.
load("@npm_angular_dev_infra_private//browsers:browser_repositories.bzl", _dev_infra_browser_repositories = "browser_repositories")

_dev_infra_browser_repositories()
7 changes: 5 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"yarn": ">= 1.0.0"
},
"scripts": {
"postinstall": "node tools/postinstall/apply-patches.js && ngcc --properties main --create-ivy-entry-points && node tools/postinstall/update-ngcc-main-fields.js",
"postinstall": "node tools/postinstall/apply-patches.js && ngcc --properties module main --create-ivy-entry-points && node tools/postinstall/update-ngcc-main-fields.js",
"build": "node ./scripts/build-packages-dist.js",
"bazel:buildifier": "find . -type f \\( -name \"*.bzl\" -or -name WORKSPACE -or -name BUILD -or -name BUILD.bazel \\) ! -path \"*/node_modules/*\" | xargs buildifier -v --warnings=attr-cfg,attr-license,attr-non-empty,attr-output-default,attr-single-file,constant-glob,ctx-args,depset-iteration,depset-union,dict-concatenation,duplicated-name,filetype,git-repository,http-archive,integer-division,load,load-on-top,native-build,native-package,output-group,package-name,package-on-top,redefined-variable,repository-name,same-origin-load,string-iteration,unused-variable,unsorted-dict-items,out-of-order-load",
"bazel:format-lint": "yarn -s bazel:buildifier --lint=warn --mode=check",
Expand Down Expand Up @@ -66,11 +66,13 @@
"zone.js": "~0.10.3"
},
"devDependencies": {
"@angular-devkit/build-optimizer": "^0.1000.0-rc.2",
"@angular-devkit/core": "^10.0.0-rc.2",
"@angular-devkit/schematics": "^10.0.0-rc.2",
"@angular/bazel": "^10.0.0-rc.2",
"@angular/benchpress": "^0.2.0",
"@angular/compiler-cli": "^10.0.0-rc.2",
"@angular/dev-infra-private": "https://github.com/angular/dev-infra-private-builds.git#2ac83eb462cb25c46a761d34dec030e360055016",
"@angular/dev-infra-private": "https://github.com/angular/dev-infra-private-builds.git#54a5865a219e89202d6ca128775e6a2489b9dac1",
"@angular/platform-browser-dynamic": "^10.0.0-rc.2",
"@angular/platform-server": "^10.0.0-rc.2",
"@angular/router": "^10.0.0-rc.2",
Expand All @@ -80,6 +82,7 @@
"@bazel/jasmine": "^1.6.0",
"@bazel/karma": "^1.6.0",
"@bazel/protractor": "^1.6.0",
"@bazel/terser": "^1.4.1",
"@bazel/typescript": "^1.6.0",
"@firebase/app-types": "^0.3.2",
"@octokit/rest": "16.28.7",
Expand Down
23 changes: 23 additions & 0 deletions test/benchmarks/material/checkbox/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
load("@npm_angular_dev_infra_private//benchmark/component_benchmark:component_benchmark.bzl", "component_benchmark")

# TODO(wagnermaciel): Update this target to provide indigo-pink in a way that doesn't require having to import it with
# stylesUrls inside the components once `component_benchmark` supports asset injection.

component_benchmark(
name = "benchmark",
driver = ":checkbox.perf-spec.ts",
driver_deps = [
"@npm//@angular/dev-infra-private",
"@npm//protractor",
"@npm//@types/jasmine",
],
ng_deps = [
"@npm//@angular/core",
"@npm//@angular/platform-browser",
"//src/material/checkbox",
"//src/cdk/a11y",
],
ng_srcs = [":app.module.ts"],
prefix = "",
styles = ["//src/material/prebuilt-themes:indigo-pink"],
)
56 changes: 56 additions & 0 deletions test/benchmarks/material/checkbox/app.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {A11yModule} from '@angular/cdk/a11y';
import {Component, NgModule, ViewEncapsulation} from '@angular/core';
import {BrowserModule} from '@angular/platform-browser';
import {MatCheckboxModule} from '@angular/material/checkbox';

/**
* @title Checkbox benchmark component.
*/
@Component({
selector: 'app-root',
template: `
<button id="show" (click)="show()">Show</button>
<button id="hide" (click)="hide()">Hide</button>
<button id="indeterminate" (click)="indeterminate()">Indeterminate</button>

<ng-container *ngIf="isVisible">
<mat-checkbox
[checked]="isChecked"
[indeterminate]="isIndeterminate"
(change)="toggleIsChecked()">
Check me!</mat-checkbox>
</ng-container>
`,
encapsulation: ViewEncapsulation.None,
styleUrls: ['//src/material/core/theming/prebuilt/indigo-pink.css'],
Copy link
Member

Choose a reason for hiding this comment

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

Did you confirm if that actually works? I wouldn't know of any resolution code that supports Bazel labels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@devversion Yeah, it works. Removing it breaks the styles. Kinda strange now that you mention it... @jelbourn Does styleUrls supports Bazel labels? lol

Copy link
Member

Choose a reason for hiding this comment

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

I have no idea how this would work. @devversion and @wagnermaciel could you pair to figure out the best way to set this up?

Copy link
Member

@devversion devversion Jun 4, 2020

Choose a reason for hiding this comment

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

Did you think about the impact of using styleUrls for that? couldn't it affect benchmarks in creation phase as the theme styles are actually loading deferred as part of the runtime JS while we'd be actually only interested in the test component in isolation?

Copy link
Member

Choose a reason for hiding this comment

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

Just checked. It works only with ngtsc (so not with View Engine) because ngtsc normalizes the path so that double-slash (as per Bazel label) becomes a path. That path is then matched against rootDirs of the TS program. bazel-out is a root directory, so the prebuilt theme can be resolved. That is reasonable (with the downside of not working for --config=view-engine).

Regardless of that though, I think we should not load the theme as part of the component factory. That will cause deviations in benchmarks as the theme will be loaded deferred and through Angular. I still think we should implement asset injection.

I think we could merge this in the interim. Asset injection would require upstream changes in dev-infra. @wagnermaciel Happy to jump on a call to explain what I refer to with asset injection

Copy link
Member

Choose a reason for hiding this comment

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

That explains it! Yeah, I agree that we can leave this for now and change it to an asset in a follow-up. @wagnermaciel can you add a TODO and file an issue for yourself?

Copy link
Contributor Author

@wagnermaciel wagnermaciel Jun 4, 2020

Choose a reason for hiding this comment

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

Done and done. Here's the todo and I created this issue in angular/angular.

})
export class CheckboxBenchmarkApp {
isChecked = false;
isVisible = false;
isIndeterminate = false;

show() { this.isVisible = true; }
hide() { this.isVisible = false; }
indeterminate() { this.isIndeterminate = true; }
toggleIsChecked() { this.isChecked = !this.isChecked; }
}


@NgModule({
declarations: [CheckboxBenchmarkApp],
imports: [
A11yModule,
BrowserModule,
MatCheckboxModule,
],
providers: [],
bootstrap: [CheckboxBenchmarkApp]
})
export class AppModule {}
105 changes: 105 additions & 0 deletions test/benchmarks/material/checkbox/checkbox.perf-spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {$, browser} from 'protractor';
import {runBenchmark} from '@angular/dev-infra-private/benchmark/driver-utilities';

describe('checkbox overview performance benchmarks', () => {
beforeAll(() => {
browser.rootEl = '#root';
});

it('renders a checked checkbox', async() => {
await runBenchmark({
id: 'checkbox-overview-render-checked',
url: '',
ignoreBrowserSynchronization: true,
params: [],
setup: async () => {
await $('#show').click();
await $('mat-checkbox').click();
},
prepare: async () => {
expect(await $('mat-checkbox input').isSelected())
.toBe(true, 'The checkbox should be in a selected state.');
await $('#hide').click();
},
work: async () => await $('#show').click()
});
});

it('renders an unchecked checkbox', async() => {
await runBenchmark({
id: 'checkbox-overview-render-unchecked',
url: '',
ignoreBrowserSynchronization: true,
params: [],
setup: async() => await $('#show').click(),
prepare: async () => {
expect(await $('mat-checkbox input').isSelected())
.toBe(false, 'The checkbox should be in an unselected state.');
await $('#hide').click();
},
work: async () => await $('#show').click()
});
});

it('renders an indeterminate checkbox', async() => {
await runBenchmark({
id: 'checkbox-overview-render-indeterminate',
url: '',
ignoreBrowserSynchronization: true,
params: [],
setup: async() => {
await $('#show').click();
await $('#indeterminate').click();
},
prepare: async () => {
expect(await $('mat-checkbox input').getAttribute('indeterminate'))
.toBe('true', 'The checkbox should be in an indeterminate state');
await $('#hide').click();
},
work: async () => await $('#show').click()
});
});

it('updates from unchecked to checked', async() => {
await runBenchmark({
id: 'checkbox-overview-click-unchecked-to-checked',
url: '',
ignoreBrowserSynchronization: true,
params: [],
setup: async () => {
await $('#show').click();
await $('mat-checkbox').click();
},
prepare: async () => {
await $('mat-checkbox').click();
expect(await $('mat-checkbox input').isSelected())
.toBe(false, 'The checkbox should be in an unchecked state.');
},
work: async () => await $('mat-checkbox').click(),
});
});

it('updates from checked to unchecked', async() => {
await runBenchmark({
id: 'checkbox-overview-click-checked-to-unchecked',
url: '',
ignoreBrowserSynchronization: true,
params: [],
setup: async () => await $('#show').click(),
prepare: async () => {
await $('mat-checkbox').click();
expect(await $('mat-checkbox input').isSelected())
.toBe(true, 'The checkbox should be in a checked state.');
},
work: async () => await $('mat-checkbox').click(),
});
});
});
Loading