fix(view_factory): fix caching of views

Previous implementation had bugs, and did not cache per ProtoView.

modules/angular2/src/core/application.js

@@ -96,7 +96,7 @@ function _injectorBindings(appComponentType): List<Binding> {
         (capacity, eventManager, shadowDomStrategy) => new rvf.ViewFactory(capacity, eventManager, shadowDomStrategy),
         [rvf.VIEW_POOL_CAPACITY, EventManager, ShadowDomStrategy]
       ),
-      bind(rvf.VIEW_POOL_CAPACITY).toValue(100000),
+      bind(rvf.VIEW_POOL_CAPACITY).toValue(10000),
       ProtoViewFactory,
       // TODO(tbosch): We need an explicit factory here, as
       // we are getting errors in dart2js with mirrors...
@@ -104,7 +104,7 @@ function _injectorBindings(appComponentType): List<Binding> {
         (capacity) => new ViewFactory(capacity),
         [VIEW_POOL_CAPACITY]
       ),
-      bind(VIEW_POOL_CAPACITY).toValue(100000),
+      bind(VIEW_POOL_CAPACITY).toValue(10000),
       Compiler,
       CompilerCache,
       TemplateResolver,

modules/angular2/src/core/compiler/view_factory.js

@@ -12,37 +12,38 @@ export const VIEW_POOL_CAPACITY = 'ViewFactory.viewPoolCapacity';
 
 @Injectable()
 export class ViewFactory {
-  _poolCapacity:number;
+  _poolCapacityPerProtoView:number;
-  _pooledViews:List<viewModule.AppView>;
+  _pooledViewsPerProtoView:Map<vieModule.ProtoView, List<viewModule.AppView>>;
 
-  constructor(@Inject(VIEW_POOL_CAPACITY) capacity) {
+  constructor(@Inject(VIEW_POOL_CAPACITY) poolCapacityPerProtoView) {
-    this._poolCapacity = capacity;
+    this._poolCapacityPerProtoView = poolCapacityPerProtoView;
-    this._pooledViews = ListWrapper.create();
+    this._pooledViewsPerProtoView = MapWrapper.create();
   }
 
   getView(protoView:viewModule.AppProtoView):viewModule.AppView {
-    // TODO(tbosch): benchmark this scanning of views and maybe
+    var pooledViews = MapWrapper.get(this._pooledViewsPerProtoView, protoView);
-    // replace it with a fancy LRU Map/List combination...
+    if (isPresent(pooledViews)) {
-    var view;
+      var result = ListWrapper.removeLast(pooledViews);
-    for (var i=this._pooledViews.length-1; i>=0; i--) {
+      if (pooledViews.length === 0) {
-      var pooledView = this._pooledViews[i];
+        MapWrapper.delete(this._pooledViewsPerProtoView, protoView);
-      if (pooledView.proto === protoView) {
-        view = ListWrapper.removeAt(this._pooledViews, i);
       }
+      return result;
     }
-    if (isBlank(view)) {
+    return this._createView(protoView);
-      view = this._createView(protoView);
-    }
-    return view;
   }
 
   returnView(view:viewModule.AppView) {
     if (view.hydrated()) {
       throw new BaseException('Only dehydrated Views can be put back into the pool!');
     }
-    ListWrapper.push(this._pooledViews, view);
+    var protoView = view.proto;
-    while (this._pooledViews.length > this._poolCapacity) {
+    var pooledViews = MapWrapper.get(this._pooledViewsPerProtoView, protoView);
-      ListWrapper.removeAt(this._pooledViews, 0);
+    if (isBlank(pooledViews)) {
+      pooledViews = [];
+      MapWrapper.set(this._pooledViewsPerProtoView, protoView, pooledViews);
+    }
+    if (pooledViews.length < this._poolCapacityPerProtoView) {
+      ListWrapper.push(pooledViews, view);
     }
   }
 

modules/angular2/src/render/dom/view/view_factory.js

@@ -18,41 +18,43 @@ export const VIEW_POOL_CAPACITY = 'render.ViewFactory.viewPoolCapacity';
 
 @Injectable()
 export class ViewFactory {
-  _poolCapacity:number;
+  _poolCapacityPerProtoView:number;
-  _pooledViews:List<viewModule.RenderView>;
+  _pooledViewsPerProtoView:Map<pvModule.RenderProtoView, List<viewModule.RenderView>>;
   _eventManager:EventManager;
   _shadowDomStrategy:ShadowDomStrategy;
 
-  constructor(@Inject(VIEW_POOL_CAPACITY) capacity, eventManager:EventManager, shadowDomStrategy:ShadowDomStrategy) {
+  constructor(@Inject(VIEW_POOL_CAPACITY) poolCapacityPerProtoView,
-    this._poolCapacity = capacity;
+      eventManager:EventManager, shadowDomStrategy:ShadowDomStrategy) {
-    this._pooledViews = ListWrapper.create();
+    this._poolCapacityPerProtoView = poolCapacityPerProtoView;
+    this._pooledViewsPerProtoView = MapWrapper.create();
     this._eventManager = eventManager;
     this._shadowDomStrategy = shadowDomStrategy;
   }
 
   getView(protoView:pvModule.RenderProtoView):viewModule.RenderView {
-    // TODO(tbosch): benchmark this scanning of views and maybe
+    var pooledViews = MapWrapper.get(this._pooledViewsPerProtoView, protoView);
-    // replace it with a fancy LRU Map/List combination...
+    if (isPresent(pooledViews)) {
-    var view;
+      var result = ListWrapper.removeLast(pooledViews);
-    for (var i=this._pooledViews.length-1; i>=0; i--) {
+      if (pooledViews.length === 0) {
-      var pooledView = this._pooledViews[i];
+        MapWrapper.delete(this._pooledViewsPerProtoView, protoView);
-      if (pooledView.proto === protoView) {
-        view = ListWrapper.removeAt(this._pooledViews, i);
       }
+      return result;
     }
-    if (isBlank(view)) {
+    return this._createView(protoView);
-      view = this._createView(protoView);
-    }
-    return view;
   }
 
   returnView(view:viewModule.RenderView) {
     if (view.hydrated()) {
       view.dehydrate();
     }
-    ListWrapper.push(this._pooledViews, view);
+    var protoView = view.proto;
-    while (this._pooledViews.length > this._poolCapacity) {
+    var pooledViews = MapWrapper.get(this._pooledViewsPerProtoView, protoView);
-      ListWrapper.removeAt(this._pooledViews, 0);
+    if (isBlank(pooledViews)) {
+      pooledViews = [];
+      MapWrapper.set(this._pooledViewsPerProtoView, protoView, pooledViews);
+    }
+    if (pooledViews.length < this._poolCapacityPerProtoView) {
+      ListWrapper.push(pooledViews, view);
     }
   }
 

modules/angular2/test/core/compiler/view_factory_spec.js

@@ -0,0 +1,73 @@
+import {describe, ddescribe, it, iit, xit, xdescribe, expect, beforeEach, el} from 'angular2/test_lib';
+
+import {ViewFactory} from 'angular2/src/core/compiler/view_factory';
+import {AppProtoView, AppView} from 'angular2/src/core/compiler/view';
+import {dynamicChangeDetection} from 'angular2/change_detection';
+
+export function main() {
+  function createViewFactory({capacity}):ViewFactory {
+    return new ViewFactory(capacity);
+  }
+
+  function createPv() {
+    return new AppProtoView(null,
+      null,
+      dynamicChangeDetection.createProtoChangeDetector('dummy', null));
+  }
+
+  describe('RenderViewFactory', () => {
+    it('should create views', () => {
+      var pv = createPv();
+      var vf = createViewFactory({
+        capacity: 1
+      });
+      expect(vf.getView(pv) instanceof AppView).toBe(true);
+    });
+
+    describe('caching', () => {
+
+      it('should support multiple AppProtoViews', () => {
+        var capacity;
+        var pv1 = createPv();
+        var pv2 = createPv();
+        var vf = createViewFactory({ capacity: 2 });
+        var view1 = vf.getView(pv1);
+        var view2 = vf.getView(pv2);
+        vf.returnView(view1);
+        vf.returnView(view2);
+
+        expect(vf.getView(pv1)).toBe(view1);
+        expect(vf.getView(pv2)).toBe(view2);
+      });
+
+      it('should reuse the newest view that has been returned', () => {
+        var capacity;
+        var pv = createPv();
+        var vf = createViewFactory({ capacity: 2 });
+        var view1 = vf.getView(pv);
+        var view2 = vf.getView(pv);
+        vf.returnView(view1);
+        vf.returnView(view2);
+
+        expect(vf.getView(pv)).toBe(view2);
+      });
+
+      it('should not add views when the capacity has been reached', () => {
+        var capacity;
+        var pv = createPv();
+        var vf = createViewFactory({ capacity: 2 });
+        var view1 = vf.getView(pv);
+        var view2 = vf.getView(pv);
+        var view3 = vf.getView(pv);
+        vf.returnView(view1);
+        vf.returnView(view2);
+        vf.returnView(view3);
+
+        expect(vf.getView(pv)).toBe(view2);
+        expect(vf.getView(pv)).toBe(view1);
+      });
+
+    });
+
+  });
+}

modules/angular2/test/render/dom/view/view_factory_spec.js

@@ -0,0 +1,75 @@
+import {describe, ddescribe, it, iit, xit, xdescribe, expect, beforeEach, el} from 'angular2/test_lib';
+
+import {ViewFactory} from 'angular2/src/render/dom/view/view_factory';
+import {RenderProtoView} from 'angular2/src/render/dom/view/proto_view';
+import {RenderView} from 'angular2/src/render/dom/view/view';
+
+export function main() {
+  function createViewFactory({capacity}):ViewFactory {
+    return new ViewFactory(capacity, null, null);
+  }
+
+  function createPv() {
+    return new RenderProtoView({
+      element: el('<div></div>'),
+      isRootView: false,
+      elementBinders: []
+    });
+  }
+
+  describe('RenderViewFactory', () => {
+    it('should create views', () => {
+      var pv = createPv();
+      var vf = createViewFactory({
+        capacity: 1
+      });
+      expect(vf.getView(pv) instanceof RenderView).toBe(true);
+    });
+
+    describe('caching', () => {
+
+      it('should support multiple RenderProtoViews', () => {
+        var capacity;
+        var pv1 = createPv();
+        var pv2 = createPv();
+        var vf = createViewFactory({ capacity: 2 });
+        var view1 = vf.getView(pv1);
+        var view2 = vf.getView(pv2);
+        vf.returnView(view1);
+        vf.returnView(view2);
+
+        expect(vf.getView(pv1)).toBe(view1);
+        expect(vf.getView(pv2)).toBe(view2);
+      });
+
+      it('should reuse the newest view that has been returned', () => {
+        var capacity;
+        var pv = createPv();
+        var vf = createViewFactory({ capacity: 2 });
+        var view1 = vf.getView(pv);
+        var view2 = vf.getView(pv);
+        vf.returnView(view1);
+        vf.returnView(view2);
+
+        expect(vf.getView(pv)).toBe(view2);
+      });
+
+      it('should not add views when the capacity has been reached', () => {
+        var capacity;
+        var pv = createPv();
+        var vf = createViewFactory({ capacity: 2 });
+        var view1 = vf.getView(pv);
+        var view2 = vf.getView(pv);
+        var view3 = vf.getView(pv);
+        vf.returnView(view1);
+        vf.returnView(view2);
+        vf.returnView(view3);
+
+        expect(vf.getView(pv)).toBe(view2);
+        expect(vf.getView(pv)).toBe(view1);
+      });
+
+    });
+
+  });
+}

modules/examples/src/hello_world/index_static.js

@@ -202,7 +202,7 @@ function setup() {
   });
 
   reflector.registerType(rvf.VIEW_POOL_CAPACITY, {
-    "factory": () => 100000,
+    "factory": () => 10000,
     "parameters": [],
     "annotations": []
   });
@@ -222,7 +222,7 @@ function setup() {
   });
 
   reflector.registerType(VIEW_POOL_CAPACITY, {
-    "factory": () => 100000,
+    "factory": () => 10000,
     "parameters": [],
     "annotations": []
   });
@@ -261,7 +261,7 @@ function setup() {
   });
 
   reflector.registerType(rvf.VIEW_POOL_CAPACITY, {
-    "factory": () => 100000,
+    "factory": () => 10000,
     "parameters": [],
     "annotations": []
   });
@@ -281,7 +281,7 @@ function setup() {
   });
 
   reflector.registerType(VIEW_POOL_CAPACITY, {
-    "factory": () => 100000,
+    "factory": () => 10000,
     "parameters": [],
     "annotations": []
   });