Bug 174252

Summary: Speedometer 2.0: Document es2015-babel-webpack build process
Product: WebKit Reporter: Mathias Bynens <mathias>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: addyo, buildbot, cdumez, commit-queue, mathias, rniwa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 172339    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews116 for mac-elcapitan
none
Patch
none
Results before and after the patch none

Description Mathias Bynens 2017-07-07 04:44:59 PDT
There is no documentation on how the build files are generated, which makes it hard to update them now and in the future. Let’s fix that.
Comment 1 Mathias Bynens 2017-07-07 04:49:12 PDT
Created attachment 314830 [details]
Patch
Comment 2 Mathias Bynens 2017-07-07 04:50:59 PDT
The main items to review are README.md, package.json, and index.html. Everything in `dist/` is the result of running the now-documented build steps.
Comment 3 Build Bot 2017-07-07 07:27:10 PDT
Comment on attachment 314830 [details]
Patch

Attachment 314830 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4070068

New failing tests:
fast/workers/dedicated-worker-lifecycle.html
Comment 4 Build Bot 2017-07-07 07:27:11 PDT
Created attachment 314846 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 5 Ryosuke Niwa 2017-07-08 01:08:15 PDT
Why is the actual benchmark content changing?
Comment 6 Mathias Bynens 2017-07-08 06:22:32 PDT
Everything in `dist/` is the result of running the now-documented build steps. You can reproduce these versions locally by following the build steps.
Comment 7 Ryosuke Niwa 2017-07-08 20:40:41 PDT
(In reply to Mathias Bynens from comment #6)
> Everything in `dist/` is the result of running the now-documented build
> steps. You can reproduce these versions locally by following the build steps.

What I'm asking is rather how your new build step is different from what we had earlier.

If there is any configuration change, I'd have to manually run the test and verify each test step is functioning correctly.
Comment 8 Mathias Bynens 2017-07-09 02:28:59 PDT
The (In reply to Ryosuke Niwa from comment #7)
> (In reply to Mathias Bynens from comment #6)
> > Everything in `dist/` is the result of running the now-documented build
> > steps. You can reproduce these versions locally by following the build steps.
> 
> What I'm asking is rather how your new build step is different from what we
> had earlier.
> 
> If there is any configuration change, I'd have to manually run the test and
> verify each test step is functioning correctly.

The configuration for this test is defined in `package.json` and `webpack.config.babel.js`. Of those two files, this patch only changes `package.json` by removing some unnecessary entries.

In `package.json`, the `dependencies`/`devDependencies` used for building `dist/*` are pinned to specific versions (e.g. `1.2.3` instead of `~1.2.3` or `^1.2.3`). This patch doesn’t change those versions.

So, the configuration has not changed, and assuming these build steps match how the previous `dist/*` output was generated (Addy?), the build process has not changed either.

The only difference is the generated chunk hashes and some variable names in the minified output. Why did these change? The only reason I can think of is that the dependencies of those dependencies might have gotten updates since the last time `dist/*` was created. For example, if webpack v1.0.0 has a dependency `"foo-bar": "^1.0.0"` in its `package.json`, running `npm install webpack@1.0.0` might install `foo-bar@1.0.0` at one point in time, but `foo-bar@1.2.3` a couple of weeks later. I suspect this is what’s causing the slight difference in output. If we want deterministic `npm install`s, we should check in `package-lock.json` files. Would you like me to do that?
Comment 9 Ryosuke Niwa 2017-07-09 13:29:07 PDT
Okay. You can codify the specific versions of each package as that would make build process more deterministic.

Could you run your version of tests as well as the one in the repository on Safari, Chrome, & Firefox to make sure there is no score difference between the two?

You can use InteractiveRunner.html to run just a single test. You probably need 5-6 samples per browser in a quiet system. Due to https://trac.webkit.org/changeset/218910, please make sure nothing is listed under System Preferences > Security & Privacy > Privacy > Accessibility. Otherwise the measurements on Safari is useless.
Comment 10 Mathias Bynens 2017-07-10 04:46:57 PDT
Created attachment 314973 [details]
Patch
Comment 11 Mathias Bynens 2017-07-10 04:49:54 PDT
(In reply to Ryosuke Niwa from comment #9)
> Okay. You can codify the specific versions of each package as that would
> make build process more deterministic.

Done, by adding a package-lock.json in the updated patch.
 
> Could you run your version of tests as well as the one in the repository on
> Safari, Chrome, & Firefox to make sure there is no score difference between
> the two?
> 
> You can use InteractiveRunner.html to run just a single test. You probably
> need 5-6 samples per browser in a quiet system. Due to
> https://trac.webkit.org/changeset/218910, please make sure nothing is listed
> under System Preferences > Security & Privacy > Privacy > Accessibility.
> Otherwise the measurements on Safari is useless.

Thanks for the additional info. Will do and report back here.
Comment 12 Mathias Bynens 2017-07-10 05:29:59 PDT
Created attachment 314974 [details]
Results before and after the patch
Comment 13 Mathias Bynens 2017-07-13 09:47:32 PDT
Ryosuke, please take a look when you get a chance.
Comment 14 Ryosuke Niwa 2017-07-25 21:14:05 PDT
Comment on attachment 314973 [details]
Patch

Great. Thanks for posting numbers!
Comment 15 WebKit Commit Bot 2017-07-25 21:42:27 PDT
Comment on attachment 314973 [details]
Patch

Clearing flags on attachment: 314973

Committed r219903: <http://trac.webkit.org/changeset/219903>
Comment 16 WebKit Commit Bot 2017-07-25 21:42:28 PDT
All reviewed patches have been landed.  Closing bug.