Fix and refactor the benchmark suite #8066

mourner · 2019-03-22T07:31:40Z

Fixes benchmarks builds, closes #8064. Gets the uploaded build down from 10MB to 719k.

Fix failing benchmark builds on master
Make source maps external to avoid huge benchmark builds (previously, 10MB)
Turn on production mode for benchmark builds so that they're minified/unasserted (had to switch to explicit benchmark names for that because class names are mangled in minified build)
Remove broken sharing button (that we have no replacement for, remove anonymous gist dependency from benchmarks #6224)
Simplify benchmark build configs and watch scripts
Introduce separate benchmark_view build again so that React/d3 and UI logic isn't built into benchmark builds
DRY up code between styles and versions benchmarks

… external

tristen

Tested locally and both versions/styles LGTM 🍭

tristen · 2019-03-22T18:09:42Z

bench/styles/index.html

 </head>

 <body>
    <div id="benchmarks"></div>
    <script src="/bench/styles/benchmarks_generated.js"></script>
+    <script src="/bench/benchmarks_view_generated.js"></script>
+    <script>
+        window.Benchmarks.run(benchmarks);


Is the benchmarks variable provided by benchmarks_view_generated.js?

Trying to think through any consequences of calling Benchmark.run for my submodule use case of the benchmark suite. I think its probably fine but I'm just curious the reasoning here.

@tristen it's provided by index.html and benchmarks.js of each respective benchmark. Styles and versions benchmarks need different treatment here — versions benchmarks group by name in a global variable because versions are ran as separate builds (and we need to keep compat with previous benchmark builds), and styles benchmarks build the benchmarks array in the build itself (because we don't upload them, and they need additional metadata like location).

tristen · 2019-03-22T18:13:07Z

bench/benchmarks_view.js

@@ -1,12 +1,89 @@
 import React from 'react';
 import ReactDOM from 'react-dom';
 import * as d3 from 'd3';
-import Axis from './lib/axis';


I kind of wish all the React components weren't defined in one script but 🤷‍♂️

I thought it would be nice to put all UI in one file, so that there's clear separation — lib only contains logic that gets insider the different benchmarks builds, and bencharks_view encapsulates all the view logic. As a follow-up, we could split the view components again in a separate folder (e.g. view or components).

Splitting up the components may be a good opportunity to remove React too. Could be easier to do if the UI is split into smaller, discrete pieces

Sounds good! Let's follow up with splitting components in a different PR (don't want to blow up the scope of this one too much).

ryanhamley · 2019-03-22T20:02:33Z

~~The styles folder is missing a rollup config and the style benchmarks are currently only running against master~~

mourner · 2019-03-22T20:18:36Z

The styles folder is missing a rollup config

All the configs are centralized in one file (bench/rollup_config_benchmarks.js) for less duplication. I left the config file in versions (although it reuses the one above) so that CI can build it without building other benchmarks. See this commit ba571a0

mourner · 2019-03-23T14:27:33Z

Here's an example timing of loading a benchmark without cache — the first line is v0.53.1, the second is master from S3 (after merging this PR), and the third is the local bench build. It's smaller than S3 master because gzip is turned off for S3 bench uploads for some reason. We should probably look into that some time.

* release-liquid: (84 commits) v0.54.0 (take two) (#8184) Fix disappearing controls in Safari 12+ (#8193) (#8194) [docs] token refactor in docs-page-shell (#8174) (#8181) v0.54.0-beta.2 (#8166) Bugfix - removeFeatureState fails with target.id === 0 (#8150) (#8164) update one more frame after canvas source paused (#8130) (#8163) move docs dependencies to dev (#8121) (#8129) v0.54.0 (#8115) CP removeFeatureState fix (#8090) v0.54.0-beta.1 (#8084) Allow setStyle diff by default with localIdeographFontFamily on (#8081) Upgrade various (mostly dev) deps, downgrade GL (#8078) Fix default marker positioning (#8074) Use @mapbox/gazetteer for benchmark coordinates (#8063) bump react-dom to v16.3.3 to fix minor vulnerability warning Fix and refactor the benchmark suite (#8066) Add max width for popups (#7906) Extrusions: Do not try to triangulate non-polygon type features (#7685) docs fixes after the merge limit flow max_workers and upgrade to v0.95.1 (#8061) ...

mourner added 4 commits March 22, 2019 09:07

pin react-dom to 16.3.2 to fix benchmarks build

5973cff

fix benchmarks builds accidentally switching to dev mode

6405641

remove broken share button from benchmarks

379a1e5

make benchmarks production-built for real this time; make source maps…

feb4877

… external

mourner added the under development 🚧 label Mar 22, 2019

mourner added 2 commits March 22, 2019 10:25

update gl and rollup w/ plugins to hopefully fix build

18d6662

simplify benchmark build configs

668e5a1

mourner force-pushed the fix-benchmarks-build branch from 215b66f to 668e5a1 Compare March 22, 2019 08:47

mourner changed the title ~~Benchmarks build fixes~~ Fix and refactor the benchmark suite Mar 22, 2019

mourner added 7 commits March 22, 2019 11:33

move Axis to benchmarks_view

cec06e0

shuffle things around in benchmarks code

ecab292

revert some breaking stuff

1fe19b9

disable minifying bench for now

21c7693

re-enable bench minification; explicitly name benchmarks

81bac6c

cut out UI logic from benchmark builds, reintroduce benchmarks_view

a35e984

centralize benchmarks rollup configs

ba571a0

mourner requested review from ryanhamley and tristen March 22, 2019 17:40

fix lint

44a1e43

mourner removed the under development 🚧 label Mar 22, 2019

tristen approved these changes Mar 22, 2019

View reviewed changes

mourner mentioned this pull request Mar 22, 2019

remove anonymous gist dependency from benchmarks #6224

Closed

fix typo

8d30c60

add forgotted watch flag

ef5cfc9

mourner merged commit de7f77d into master Mar 23, 2019

mourner deleted the fix-benchmarks-build branch March 23, 2019 14:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix and refactor the benchmark suite #8066

Fix and refactor the benchmark suite #8066

mourner commented Mar 22, 2019 •

edited

Loading

tristen left a comment

tristen Mar 22, 2019

tristen Mar 22, 2019

mourner Mar 22, 2019

tristen Mar 22, 2019

mourner Mar 22, 2019

ryanhamley Mar 22, 2019

mourner Mar 22, 2019

ryanhamley commented Mar 22, 2019 •

edited

Loading

mourner commented Mar 22, 2019

mourner commented Mar 23, 2019

Fix and refactor the benchmark suite #8066

Fix and refactor the benchmark suite #8066

Conversation

mourner commented Mar 22, 2019 • edited Loading

tristen left a comment

Choose a reason for hiding this comment

tristen Mar 22, 2019

Choose a reason for hiding this comment

tristen Mar 22, 2019

Choose a reason for hiding this comment

mourner Mar 22, 2019

Choose a reason for hiding this comment

tristen Mar 22, 2019

Choose a reason for hiding this comment

mourner Mar 22, 2019

Choose a reason for hiding this comment

ryanhamley Mar 22, 2019

Choose a reason for hiding this comment

mourner Mar 22, 2019

Choose a reason for hiding this comment

ryanhamley commented Mar 22, 2019 • edited Loading

mourner commented Mar 22, 2019

mourner commented Mar 23, 2019

mourner commented Mar 22, 2019 •

edited

Loading

ryanhamley commented Mar 22, 2019 •

edited

Loading