krisselden / ember-macro-benchmark

Benchmark recording of an ember app with running with 2 versions of Ember.
25 stars 9 forks source link

Update readme to describe how to setup application for benchmarks #14

Closed rondale-sc closed 2 years ago

krisselden commented 7 years ago

One caveat, if you do this in your application and are deploying it you may want to ensure performance.mark exists.

rondale-sc commented 7 years ago

@krisselden Definitely. Do you think it is worth mentioning in the README?

I used as my example:

https://github.com/emberobserver/client/commit/d28f5f2a27387321c060e85477f166b9010d21be

Perhaps I could add that also?

rondale-sc commented 7 years ago

Oops. I meant: https://github.com/emberobserver/client/blob/master/app/snippets/perf-utils.js

krisselden commented 7 years ago

@rondale-sc @rwjblue this can be injected as well

if (location.search === "?trace_redirect") {
  Ember.onLoad('application', function (app) {
    app.instanceInitializer({...});
  });
}
krisselden commented 7 years ago

Wondering if we shouldn't make it an option for this to be configured to be injected like so:

if (key === htmlKey) {
  text = text.replace(`</body>`, `<script>
Ember.onLoad('application', function (app) {
  function willTransition() {
    performance.mark('willTransition');
  }

  function didTransition() {
    performance.mark('didTransition');

    requestAnimationFrame(() => {
      performance.mark('beforePaint');

      requestAnimationFrame(() => {
        performance.mark('afterPaint');

        if (location.includes('?perf.tracing')) {
          setTimeout(() => document.location.href = 'about:blank', 10);
        }
      });
    });
  }

  app.instanceInitializer({
    name: 'benchmark',
    initialize: function (instance) {
      var router = instance.lookup("router:main");
      router.on("willTransition", willTransition);
      router.on("didTransition", didTransition);
    }
  });
});
</script></body>`);
  ...
}
krisselden commented 7 years ago

The only reason I didn't do this because the app already had markers.

rondale-sc commented 7 years ago

@krisselden Would this necessitate addonizing this? Or a separate addon to do the above?

rwjblue commented 7 years ago

@rondale-sc - If I understand correctly, it shouldn't require any restructuring into an addon. I believe the idea is to configure the HAR server to modify the HTML payload itself to add the snippet @krisselden pasted above. We already have code to replace the JS assets, this should end up similar...

rondale-sc commented 7 years ago

@rwjblue Ah, that's interesting. I see. That is a great idea! Though now that I see it, we'd still have to have the beforeVendor (et, al) markers placed by consumers?

krisselden commented 7 years ago

I have a new snippet that uses EmberENV load hooks because I found that if you have an app setup to not wait for jquery ready that the load hook version above is racy.

krisselden commented 7 years ago
  if (typeof EmberENV === 'undefined') EmberENV = {};
  if (EmberENV.EMBER_LOAD_HOOKS === undefined) EmberENV.EMBER_LOAD_HOOKS = {};
  if (EmberENV.EMBER_LOAD_HOOKS.application === undefined) EmberENV.EMBER_LOAD_HOOKS.application = [];
  EmberENV.EMBER_LOAD_HOOKS.application.push(function (app) { ... });
rwjblue commented 7 years ago

Nice, thanks for sharing that @krisselden