Skip to content
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

How to concatenate assets together #13

Closed
renoirb opened this issue Oct 4, 2014 · 11 comments
Closed

How to concatenate assets together #13

renoirb opened this issue Oct 4, 2014 · 11 comments

Comments

@renoirb
Copy link
Member

renoirb commented Oct 4, 2014

While looking at what gets generated, I see that we still have numerous assets loaded. Is there a way we could minimize the number of HTTP requests by bundling assets together.

I see that the project is using something called "webassets" in Pyramids and that its possible to bundle them up together.

@tilgovi
Copy link
Contributor

tilgovi commented Oct 4, 2014

production.ini does this

@tilgovi
Copy link
Contributor

tilgovi commented Oct 4, 2014

We could be more aggressive about it and bundle more. I'm uncertain whether that's a good idea, though.

@renoirb
Copy link
Member Author

renoirb commented Oct 5, 2014

I’m running on production.ini. The run file calls production.ini instead and that’s where I saw that more than 20 javascript files are still called.

@tilgovi
Copy link
Contributor

tilgovi commented Oct 5, 2014

Should be only 2 from the embed. One JS. One CSS. The rest are in the
iframe, which should load after the rest of the page, I think. If so, I
don't know whether bundling those is good. It can decrease parallelism.
On Oct 4, 2014 5:11 PM, "Renoir Boulanger" notifications@github.com wrote:

I’m running on production.ini. The run file calls production.ini instead
and that’s where I saw that more than 20 javascript files are still called.


Reply to this email directly or view it on GitHub
#13 (comment)
.

@tilgovi
Copy link
Contributor

tilgovi commented Oct 5, 2014

I'll consider bundling more aggressively in upstream h. That's the bulk of it. Nothing to do here for now. But if you want to look at the assets.yaml in h, or webassets documentation, you can minify the auth bundle in our assets.yaml here.

@tilgovi tilgovi closed this as completed Oct 5, 2014
@renoirb
Copy link
Member Author

renoirb commented Oct 6, 2014

I still need to make this happen.

@renoirb renoirb reopened this Oct 6, 2014
@tilgovi
Copy link
Contributor

tilgovi commented Oct 6, 2014

You do realize this decreases parallelism and is no longer a universally recognized optimization. Have you profiled the difference? I've not done this in h yet because I have not.

Why do you think you just do this?

@renoirb
Copy link
Member Author

renoirb commented Oct 6, 2014

Desired end result is to have as less as possible assets —and therefore less HTTP requests— to download for each pages that will load the annotation sidebar.

That is because every request counts. Not all web apps are fast to load. MediaWiki is already slow by itself. Once we have this available to all participating W3C specs and WebPlatform web apps, we want to be sure that the notes-server can hold the load and not slow down other parties relying on it.

Maybe i’m wrong on where I want to make the desired end result, maybe I’m describing in a wrong way, but there must be a way.

@tilgovi
Copy link
Contributor

tilgovi commented Oct 6, 2014

I think you are possibly wrong about the desired end result. It may be detrimental to performance.

Copying my message from the mailing list:

  1. Far future cache headers.

Running with production.ini, headers are set for far future expirations, so browsers should only have to fetch these scripts the first time the sidebar loads.

  1. Parallel downloads

Browsers these days make a great attempt to download scripts in parallel. Bundling prohibits that. With HTTP Keepalive and especially with SPDY (supported in recent Nginx) the amount of overhead for each requests drops a lot and the parallelism is even more useful.

  1. Hash-based query string cache busting

Our asset pipeline generates query strings wherever our static assets are referenced that contain a hash of the content. If we update one dependency, the browser can use the old, cached versions of all the other ones still. With bundling, the entire application and all its dependencies get invalidated if we change one thing.

Taken together, these are enough to convince me that bundling isn't obviously a good idea, and could even be a bad idea. That's why I want to profile.

@tilgovi
Copy link
Contributor

tilgovi commented Oct 6, 2014

So, we can definitely do what you're asking, but you should understand that you may not want to. It is not obvious or certain that bundling the way you describe is a good idea or would result in better performance. It may even result in worse performance.

tilgovi added a commit to hypothesis/h that referenced this issue Oct 8, 2014
Tweak the way we organize our asset bundles for fewer output files,
straightforward names, and no conflict of dev and prod output files.

- Break session and helpers into their own bundles.

- Bundle vendor libs up where they are not common to more than one
  optional component. For example, angular-resource is needed by auth
  and the app, but angular-bootstrap is only needed by helpers since
  that is where we redefine tabbable and create tab-reveal (in a new
  helpers/ui-helpers.coffee file). Another example is angular-resource,
  which is only used by the session bundle.

- Inline icomoon.css.

- Rename some outputs:
  - hypothesis-app.{css,js} -> app.{css,js}
    This is the main site application.
  - hypothesis-inject.{css,js} -> hypothesis.{css,js}
    This is the inject script.

Close #1555
Related to webplatform/annotation-service#13
tilgovi added a commit to hypothesis/h that referenced this issue Oct 8, 2014
Tweak the way we organize our asset bundles for fewer output files,
straightforward names, and no conflict of dev and prod output files.

- Break session and helpers into their own bundles.

- Bundle vendor libs up where they are not common to more than one
  optional component. For example, angular-resource is needed by auth
  and the app, but angular-bootstrap is only needed by helpers since
  that is where we redefine tabbable and create tab-reveal (in a new
  helpers/ui-helpers.coffee file). Another example is angular-resource,
  which is only used by the session bundle.

- Inline icomoon.css.

- Rename some outputs:
  - hypothesis-app.{css,js} -> app.{css,js}
    This is the main site application.
  - hypothesis-inject.{css,js} -> hypothesis.{css,js}
    This is the inject script.

Close #1555
Related to webplatform/annotation-service#13
tilgovi added a commit to hypothesis/h that referenced this issue Oct 8, 2014
Tweak the way we organize our asset bundles for fewer output files,
straightforward names, and no conflict of dev and prod output files.

- Break session and helpers into their own bundles.

- Bundle vendor libs up where they are not common to more than one
  optional component. For example, angular-resource is needed by auth
  and the app, but angular-bootstrap is only needed by helpers since
  that is where we redefine tabbable and create tab-reveal (in a new
  helpers/ui-helpers.coffee file). Another example is angular-resource,
  which is only used by the session bundle.

- Inline icomoon.css.

- Rename some outputs:
  - hypothesis-app.{css,js} -> app.{css,js}
    This is the main site application.
  - hypothesis-inject.{css,js} -> hypothesis.{css,js}
    This is the inject script.

Close #1555
Related to webplatform/annotation-service#13
tilgovi added a commit to hypothesis/h that referenced this issue Oct 8, 2014
Tweak the way we organize our asset bundles for fewer output files,
straightforward names, and no conflict of dev and prod output files.

- Break session and helpers into their own bundles.

- Bundle vendor libs up where they are not common to more than one
  optional component. For example, angular-bootstrap is only needed by
  helpers since that is where we now redefine tabbable and create
  the tab-reveal directive (in a new helpers/ui-helpers.coffee file).
  Another example is angular-resource, which is only used by the session
  bundle. On the other hand, angular-route is used by the app, but soon
  also by the auth bundle as soon as we unify all the login forms and
  boot out deform.

- Inline icomoon.css.

- Rename some outputs:
  - hypothesis-app.{css,js} -> app.{css,js}
    This is the main site application.
  - hypothesis-inject.{css,js} -> hypothesis.{css,js}
    This is the inject script.

Close #1555
Related to webplatform/annotation-service#13
tilgovi added a commit to hypothesis/h that referenced this issue Oct 8, 2014
Tweak the way we organize our asset bundles for fewer output files,
straightforward names, and no conflict of dev and prod output files.

- Break session and helpers into their own bundles.

- Bundle vendor libs up where they are not common to more than one
  optional component. For example, angular-bootstrap is only needed by
  helpers since that is where we now redefine tabbable and create
  the tab-reveal directive (in a new helpers/ui-helpers.coffee file).
  Another example is angular-resource, which is only used by the session
  bundle. On the other hand, angular-route is used by the app, but soon
  also by the auth bundle as soon as we unify all the login forms and
  boot out deform.

- Inline icomoon.css.

- Rename some outputs:
  - hypothesis-app.{css,js} -> app.{css,js}
    This is the main site application.
  - hypothesis-inject.{css,js} -> hypothesis.{css,js}
    This is the inject script.

Close #1555
Related to webplatform/annotation-service#13
@tilgovi
Copy link
Contributor

tilgovi commented Oct 22, 2014

Solved upstream in hypothesis/h@f5c8d47

@tilgovi tilgovi closed this as completed Oct 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants