Bug 175926 – Speedometer: Restructure Ember example

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Mathias Bynens]

Speedometer: Remove unused stylesheet in Ember example

[Mathias Bynens]

Created attachment 318971 [details]
Patch

[Mathias Bynens]

I know the diff looks massive (because of the generated + minified files), but don’t despair! Only the hashes changed.

E.g. the only difference in todomvc-*.js is the `version` hash at the bottom. Old version:

    version: "0.0.0+164d27e2"

New version:

    version: "0.0.0+a2ca1650"

[Mathias Bynens]

Created attachment 318972 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 318972 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=318972&action=review

> PerformanceTests/ChangeLog:10
> +        * Speedometer/resources/todomvc/architecture-examples/emberjs/source/app/index.html: Removed stylesheet reference.
> +        * Speedometer/resources/todomvc/architecture-examples/emberjs/source/app/styles/.gitkeep: Added.
> +        * Speedometer/resources/todomvc/architecture-examples/emberjs/source/dist/*: Regenerated per build instructions.

Something is not right. tests.js refer to todomvc/architecture-examples/emberjs/index.html yet this is re-generating emberjs/source/dist/index.html:

Suites.push({
    name: 'EmberJS-TodoMVC',
    url: 'todomvc/architecture-examples/emberjs/index.html',

[Mathias Bynens]

Created attachment 318976 [details]
Patch

[Mathias Bynens]

(In reply to Ryosuke Niwa from comment #4)
> Comment on attachment 318972 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318972&action=review
> 
> > PerformanceTests/ChangeLog:10
> > +        * Speedometer/resources/todomvc/architecture-examples/emberjs/source/app/index.html: Removed stylesheet reference.
> > +        * Speedometer/resources/todomvc/architecture-examples/emberjs/source/app/styles/.gitkeep: Added.
> > +        * Speedometer/resources/todomvc/architecture-examples/emberjs/source/dist/*: Regenerated per build instructions.
> 
> Something is not right. tests.js refer to
> todomvc/architecture-examples/emberjs/index.html yet this is re-generating
> emberjs/source/dist/index.html:
> 
> Suites.push({
>     name: 'EmberJS-TodoMVC',
>     url: 'todomvc/architecture-examples/emberjs/index.html',

Good catch. That should’ve been `todomvc/architecture-examples/emberjs/source/dist/index.html` all along.

EmberJS is the only test with such a weird directory structure: `/emberjs` is pretty much empty, `/emberjs/source` contains the actual source, but the output is written to `/emberjs/source/dist`. The new patch fixes that as well by getting rid of the intermediate `source` folder.

[Mathias Bynens]

Created attachment 318978 [details]
Results before and after applying the patch

[Ryosuke Niwa]

Okay, this results in a dramatic performance improvements in both Safari & Chrome. The magnitude of the change here is almost 4x. I don't think we can accept this change as is.

== Before ==

Safari
34.6
34.9
34.7

Chrome
28.0
28.4
28.1

== After ==

Safari
108
107
109

Chrome
109
107
110

[Ryosuke Niwa]

Comment on attachment 318976 [details]
Patch

Now that https://trac.webkit.org/changeset/221205 has been landed, we can land this.

[WebKit Commit Bot]

Comment on attachment 318976 [details]
Patch

Clearing flags on attachment: 318976

Committed r221206: <http://trac.webkit.org/changeset/221206>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34089824>