ember-cli / broccoli-builder

MIT License
1 stars 10 forks source link

[do-not-merge] beta uber-cache #30

Open lifeart opened 6 years ago

lifeart commented 6 years ago

Windows Rebuild times 3-4x faster! (hooky caching)

Cache will work after second rebuild!


take UBER_CACHE object from this PR and add it after this code

RSVP.EventTarget.mixin(Builder.prototype)

find

Builder.prototype.build = function (willReadStringTree)

replace to

Builder.prototype.build = function (willReadStringTree, resultAnnotation)

and before

  var builder = this

  var newTreesRead = []
  var nodeCache = []
  this.isThrown = false;

add

UBER_CACHE.setStrategy(resultAnnotation);

and before

readTreeRunning = true

add

let cachedItem = UBER_CACHE.getItemFromCache(subtree);
            if (cachedItem) {
           node.addChild(cachedItem);
           return cachedItem.directory;
         }

and after

.then(function (childNode)

add

UBER_CACHE.addItemToCache(subtree, childNode);

`` done! Second rebuild gonna be faster 3x times!

lifeart commented 6 years ago

before

Build successful (7400ms) – Serving on http://localhost:4200/

Slowest Nodes (totalTime => 5% )              | Total (avg)
----------------------------------------------+---------------------
Funnel (44)                                   | 882ms (20 ms)
SimpleConcatConcat: Vendor JS (1)             | 811ms
SassCompiler (1)                              | 711ms
broccoli-persistent-filter:Babel > [Ba... (2) | 569ms (284 ms)
broccoli-persistent-filter:SimpleReplace (3)  | 567ms (189 ms)
Funnel: Addon JS (51)                         | 564ms (11 ms)
SimpleConcatConcat: Concat App (1)            | 517ms
SimpleConcatConcat: Concat: Vendor Sty... (1) | 477ms

after

Build successful (2166ms) – Serving on http://localhost:4200/

Slowest Nodes (totalTime => 5% )              | Total (avg)
----------------------------------------------+---------------------
broccoli-persistent-filter:Babel > [Ba... (1) | 747ms
SimpleConcatConcat: Concat App (1)            | 519ms
TreeMerger (preprocessedApp & templates) (1)  | 284ms
Funnel: Filtered App (1)                      | 227ms
TreeMerger (app) (1)                          | 149ms
broccoli-persistent-filter:TemplateCom... (1) | 140ms
lifeart commented 6 years ago

I think we can add some middleware hooks to support tree-caching approach on application level, instead of system.

Going to extract UBER_CACHE into https://github.com/lifeart/broccoli-memento

Also, I think we can add cache abstraction, dependent from "where this tree was created".

I know, this caching approach working well only for "stupid" addons, for "smart" addons we can create some ignore rule.

serproqnx commented 6 years ago

2 rebuild 4 times faster. nice

knownasilya commented 6 years ago

Before: 830ms - 930ms After: 270ms - 310ms

Using Ember CLI ~3.1.4

knownasilya commented 6 years ago

I think this change causes the following error on test rebuilds:

Uncaught Error: Could not find module `@em
ber/test-helpers` imported from `map/tests
/test-helper` at http://localhost:7357/ass
ets/vendor.js, line 252
Uncaught Error: Assertion Failed: The test
s file was not loaded. Make sure your test
s index.html includes "assets/tests.js". a
t http://localhost:7357/assets/vendor.js,
line 32362
lifeart commented 6 years ago

@knownasilya yeah, it may be, you can add tests tree to cache ignore (like ember-data) excludedAnnotationTreesPaths

stefanpenner commented 6 years ago

This seems related/overlapping/similar to: https://github.com/broccolijs/broccoli-plugin/pull/23

TL;DR if plugins are pure (no side-affects if their inputs don't change) broccoli can very easily skip a-bunch of work on rebuilds. For example, with this model, given A -> B -> C, broccoli can simply skip C and B entirely, if A didn't change. Also, if the watcher doesn't mark A as dirty, broccoli can simply skip all three steps.

That effort was paused until 1.x (now 2.x broccoli) lands, so that the watcher is part of broccoli proper, allowing for watcher to mark the broccoli roots as dirty, thus skipping work. We noticed in testing, without skipping roots, the wins where quite low.

I am obviously biased, but I feel the linked work is a more resilient approach (less edge cases). But obviously a tad more work, as many plugins will need to be upgraded (and some final cleanup work within broccoli proper). I would love some help with ^.

If I misunderstood this PR, please share. Otherwise I would love to hear pros/cons of the two approaches, and if you see a path forward.

lifeart commented 6 years ago

@knownasilya try

find:

return [has, key];

add:


 if (String(key).toLowerCase().includes('test')) {
      return [false, key];
 }
return [has, key];
lifeart commented 6 years ago

@stefanpenner, yes this RP has more "rough" appoach for cachig.

As I can see, for this moment we have about 90% "side-effects-free" addons, and other 10% can be optimized.

Current problem: working with ember-cli on windows - painfully slow

I can see list of improvements for Broccoli 2.0 and future Ember-cli releases, but, I can't wait for it, also, it don't solve problems for older ember-cli versions.

If we taking a look at "exsting" projects - we can optimize rebuild times using this hook, with manual ignore for "side-effected" addons.

I don't expect that this PR has been merged, because without https://github.com/ember-cli/ember-cli/pull/7891/files it will be less usable, and cli changes wouldn't merged because this part of code under refactoring for broccoli 2 support.

But, I wan to keep this kinda hack, for users with same pain and legacy ember-cli. I don't think what fork is more usable for it, let it be this PR, to let users apply hack "manually".

Good things in future. But I need faster rebuilds now, not in futute. And this solution must be portable for different ember projects.

stefanpenner commented 6 years ago

@lifeart sounds good, I like your approach!

Do I take your comment as agreement, that the side-affect free approach will eventually supersede this? Do you have any concerns about that (ofcourse other then it isn't complete itself yet).

Also, finally would you be interesting in helping with that effort?

lifeart commented 6 years ago

@stefanpenner yes, you right! Also, i'm interesting in helping with that approach.

I'm thinking that we need uniq names for each tree, and extended meta from hooks like (treeForAddon) with information about hook name and that addon name produce it. Also, it will be nice to have an cli-provided tree naming convention with warnings.