Bug 174252

Created attachment 314830 [details] Patch

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 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

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

Why is the actual benchmark content changing?

Everything in `dist/` is the result of running the now-documented build steps. You can reproduce these versions locally by following the build steps.

(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 (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?

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.

Created attachment 314973 [details] Patch

(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.

Created attachment 314974 [details] Results before and after the patch

Ryosuke, please take a look when you get a chance.

Comment on attachment 314973 [details] Patch Great. Thanks for posting numbers!

Comment on attachment 314973 [details] Patch Clearing flags on attachment: 314973 Committed r219903: <http://trac.webkit.org/changeset/219903>

All reviewed patches have been landed. Closing bug.